Skip to content

Conversation

@Meghs21
Copy link

@Meghs21 Meghs21 commented Oct 18, 2025

This PR addresses the confusing behavior of the Axes.c2p() function when passing 1D lists or arrays as input coordinates.

Changes made:

Fix for c2p() behavior:

Properly handles single points provided as lists or 1D numpy arrays.

Returns a single point as np.array of shape (3,) and multiple points as np.array of shape (3, n_points).

Maintains compatibility with existing multi-argument input (x, y, z) and 2D list/array inputs.

Added tests:

Tests that reproduce the previously confusing outputs for 1D lists and arrays.

Confirms that single points, multi-points, and lists of points all produce expected outputs.
C2PBehavior_ManimCE_v0 19 0

Motivation:
This clarifies the expected input/output behavior of c2p(), reduces confusion for users, and ensures consistency across different input types.

Notes:

No changes to existing functionality beyond the improved handling of 1D inputs.

Thanks to @uwezi for highlighting the issue in #4073
.

@chopan050
Copy link
Contributor

chopan050 commented Oct 24, 2025

Closing this PR, because the changes seem like AI slop.

Major reasons:

  • The c2p() method is not properly indented, which prevents Manim code from even being executed.
  • The new c2p() only reshapes and pads the given coords, but removes the entire logic of "transforming those coordinates in a CoordinateSystem into the corresponding points/positions on a scene". There's no access to the self.axes, nor anything belonging to self at all. This, of course, makes multiple tests break.

Thus, there's no way that a human could have written that code and then tested it before making this PR, because it would have instantly failed.

Slighly minor reasons:

  • There's an entire manim-env folder among these changes which should not be here.
  • The method which should actually have been modified is coords_to_point(), not c2p(). c2p() is simply a short alias for coords_to_point() and its sole function is to redirect to the latter function. They should always do the same thing.
  • The tests don't actually test anything as in "assert that these points are close enough to these values" or "verify that the generated frames match". One of them does print(), which shouldn't be done in our tests, and the other one doesn't really do anything useful. They're also placed in the wrong directories and don't follow the correct format which is established in the rest of the tests.

@chopan050 chopan050 closed this Oct 24, 2025
@github-project-automation github-project-automation bot moved this from 🆕 New to ❌ Rejected in Dev Board Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Rejected

Development

Successfully merging this pull request may close these issues.

3 participants