fix(graphing): align coordinate conversion type contracts - #4951
fix(graphing): align coordinate conversion type contracts#4951emerardd wants to merge 2 commits into
Conversation
behackl
left a comment
There was a problem hiding this comment.
Hi, and thanks for contributing! It is a worthwhile task to expand the existing type hints to reflect that sequences can be input too.
However, I'd prefer if you'd write them out using the existing type aliases from manim.typing instead of writing them as these multi-unions.
For example, point could be typed as Point3DLike | Point3DLike_Array, while coord could be PointNDLike | PointNDLike_Array | Mobject.
(And while I know that this is a preexisting problem because numpy arrays are described by Point3DLike too: actually trying to multiply <numpy array> @ axes errors out because numpy interprets the @ as a matrix multiplication that effectively fails.)
Please take a look and let me know whether you agree.
|
Thanks for the review — I agree. The expanded unions duplicated the shape semantics already captured by the aliases in Updated in
I also agree that The focused coordinate tests, Ruff, targeted mypy checks, and runtime contract checks pass. Could you take another look when convenient? |
12eb8c7 to
a86a40f
Compare
Closes #4804.
Summary
CoordinateSystemandAxescoordinate-conversion type contracts with their existing single-point and batch behavior.coords_to_point,point_to_coords, theirc2p/p2caliases, and the forward/reverse@operators.np.ndarrayfor return values so the annotations cover both point batches shaped(M, 3)and component-wise batches shaped(3, M).Validation
ruff check --no-cache manim/mobject/graphing/coordinate_systems.py@directions, andThreeDAxesgit diff --check