-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add @
shorthand for CoordinateSystem
methods coords_to_point
(c2p
) and point_to_coords
(p2c
)
#3754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add @
shorthand for CoordinateSystem
methods coords_to_point
(c2p
) and point_to_coords
(p2c
)
#3754
Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this a lot! 👍
It might make sense modify or include a new example in the example gallery that uses this notation.
def __matmul__(self, coord: Point3D | Mobject): | ||
if isinstance(coord, Mobject): | ||
coord = coord.get_center() | ||
return self.coords_to_point(*coord) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return self.coords_to_point(*coord) | |
return self.coords_to_point(coord) |
?
Probably a test would be nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with removing the *
is that ax @ (1, 0, 0)
then returns a 3x3 array, which I felt was kinda unintuitive.
As for the test, I already have a doctest in Axes
. Is another test really necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the interest of getting this merged, I went ahead and added a test for the behavior.
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
I just added it to |
@
shorthand for CoordinateSystem
methods coords_to_point
(c2p
) and point_to_coords
(p2c
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Will get back to the unresolved comment soon |
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
Allows
ax @ (1, 0, 0)
forcoords_to_point
and(1, 0, 0) @ ax
forpoint_to_coords
Inspired by @abul4fia