Skip to content

Conversation

JasonGrace2282
Copy link
Member

@JasonGrace2282 JasonGrace2282 commented May 9, 2024

Allows ax @ (1, 0, 0) for coords_to_point and (1, 0, 0) @ ax for point_to_coords

Inspired by @abul4fia

@behackl behackl added the new feature Enhancement specifically adding a new feature (feature request should be used for issues instead) label May 9, 2024
Copy link
Member

@behackl behackl left a 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return self.coords_to_point(*coord)
return self.coords_to_point(coord)

?
Probably a test would be nice

Copy link
Member Author

@JasonGrace2282 JasonGrace2282 May 9, 2024

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?

Copy link
Member Author

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.

@JasonGrace2282
Copy link
Member Author

I just added it to NumberLine and changed the CoordinateSystem example to use @ notation. In the future, maybe we should transition to that being the "standard" (although c2p can always exist for more complex expressions)

@JasonGrace2282 JasonGrace2282 requested a review from Viicos May 17, 2024 22:20
@chopan050 chopan050 changed the title Add shorthand for axes Add @ shorthand for CoordinateSystem methods coords_to_point (c2p) and point_to_coords (p2c) May 20, 2024
@JasonGrace2282 JasonGrace2282 requested a review from chopan050 May 23, 2024 12:17
Copy link
Contributor

@chopan050 chopan050 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Viicos
Copy link
Member

Viicos commented May 23, 2024

Will get back to the unresolved comment soon

Co-authored-by: Victorien <[email protected]>
@JasonGrace2282 JasonGrace2282 enabled auto-merge (squash) May 27, 2024 17:18
@JasonGrace2282 JasonGrace2282 merged commit 90ae6ad into ManimCommunity:main May 27, 2024
@JasonGrace2282 JasonGrace2282 deleted the @-shorthand branch July 17, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature Enhancement specifically adding a new feature (feature request should be used for issues instead)
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants