Skip to content

Refactor Mobject.put_start_and_end_on() to shift Mobject to start when it's a closed curve#4658

Merged
chopan050 merged 12 commits intoManimCommunity:mainfrom
GoThrones:refactor/put_start_and_end_on
Apr 14, 2026
Merged

Refactor Mobject.put_start_and_end_on() to shift Mobject to start when it's a closed curve#4658
chopan050 merged 12 commits intoManimCommunity:mainfrom
GoThrones:refactor/put_start_and_end_on

Conversation

@GoThrones
Copy link
Copy Markdown
Contributor

Overview: What does this pull request change?

This PR refactors the method "put_start_and_end_on" in mobject.py and opengl_mobject.py:

Replaces self.points = np.array(start) with self.shift(np.asarray(start) - current_start) in the case of a mobject whose start and end point are the same, in both mobject.py and opengl_mobject.py

Adds a warning that warns the user that the mobject which is calling this method is a loop, and it will be shifted.

Some variable names have been changed for better reading.

Motivation and Explanation: Why and how do your changes improve the library?

The put_start_and_end_on method in both mobject.py and opengl_mobject.py previously used self.points = np.array(start) when the mobject had its start and end as the same point. This collapsed all of the mobject's points into a single Point3D, destroying its geometry entirely. For example, calling this method on a Circle or Square would reduce it to a single point, making it cease to exist visually.
This PR fixes the issue by replacing that line with self.shift(np.asarray(start) - current_start), which simply repositions the mobject to the start point while fully preserving its geometry.

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

Copy link
Copy Markdown
Member

@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.

Thanks for your contribution! Shifting the Mobject in this case makes sense, and I appreciate both expanding the variable names and making the behavior consistent in Mobject and OpenGLMobject.

I left some change requests:

Comment thread manim/mobject/mobject.py
Comment thread manim/mobject/mobject.py Outdated
Comment thread manim/mobject/opengl/opengl_mobject.py Outdated
@GoThrones GoThrones requested a review from chopan050 April 13, 2026 11:22
@GoThrones
Copy link
Copy Markdown
Contributor Author

I've made necessary changes in the code as suggested by you.

Copy link
Copy Markdown
Member

@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.

Thanks for applying the requested changes!

Now most pipelines do not fail because of that test. However, precommit is still failing for a different reason: the current use of warnings.warn(). Please address this final suggestion in order to be able to merge this:

Comment thread manim/mobject/opengl/opengl_mobject.py
Comment thread manim/mobject/mobject.py
@GoThrones
Copy link
Copy Markdown
Contributor Author

I appreciate your suggestions. Thank you.
I've rectified the code in accordance with your suggestion.

Copy link
Copy Markdown
Member

@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! Thanks for these changes!

@chopan050 chopan050 changed the title Refactor/put start and end on Refactor Mobject.put_start_and_end_on() to shift Mobject to start when it's a closed curve Apr 14, 2026
@chopan050 chopan050 enabled auto-merge (squash) April 14, 2026 19:53
@chopan050 chopan050 merged commit c457249 into ManimCommunity:main Apr 14, 2026
14 of 15 checks passed
@GoThrones GoThrones deleted the refactor/put_start_and_end_on branch April 15, 2026 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants