Refactor Mobject.put_start_and_end_on() to shift Mobject to start when it's a closed curve#4658
Conversation
chopan050
left a comment
There was a problem hiding this comment.
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:
…Thrones/ManimCE into refactor/put_start_and_end_on
for more information, see https://pre-commit.ci
…Thrones/ManimCE into refactor/put_start_and_end_on
for more information, see https://pre-commit.ci
|
I've made necessary changes in the code as suggested by you. |
chopan050
left a comment
There was a problem hiding this comment.
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:
…Thrones/ManimCE into refactor/put_start_and_end_on
|
I appreciate your suggestions. Thank you. |
chopan050
left a comment
There was a problem hiding this comment.
LGTM! Thanks for these changes!
Mobject.put_start_and_end_on() to shift Mobject to start when it's a closed curve
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