Skip to content
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

Fix incorrect assignment in Mobject.put_start_and_end_on #4008

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dark
Copy link

@dark dark commented Nov 10, 2024

Overview: What does this pull request change?

PR #3718 changed the behavior of Mobject.put_start_and_end_on when start == end, however the assigment to self.points was incorrect. The existing code assigned the Point3D directly to self.points, that then becomes an array with shape (3,), while instead it should really have shape (1, 3).

This commit fixes the incorrect code and associated test.

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

Links to added or changed documentation pages

Further Information and Comments

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

@dark dark marked this pull request as draft November 10, 2024 23:50
@dark dark force-pushed the fix-pr3718 branch 2 times, most recently from e1482dd to fe0c9f6 Compare November 10, 2024 23:56
@dark dark marked this pull request as ready for review November 11, 2024 00:03
PR ManimCommunity#3718 changed the behavior of `Mobject.put_start_and_end_on` when
`start == end`, however the assigment to `self.points` was
incorrect. The existing code assigned the Point3D directly to
`self.points`, that then becomes an array with shape `(3,)`, while
instead it should really have shape `(1, 3)`.

This commit fixes the incorrect code and associated test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

1 participant