Fix :meth:VMobject.pointwise_become_partial failing when vmobject is self

[irvanalhaq9]

EDIT by chopan050 - Original title: "Fix Line failing with buff and path_arc"

Overview: What does this pull request change?

Fixes #4132

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

Issue Explanation

When buff is given, Line internally calls pointwise_become_partial, passing self as the argument for vmobject:

self.pointwise_become_partial(vmobject=self, a=buff_proportion, b=1 - buff_proportion)

In Manim v0.19.0, the pointwise_become_partial code was modified in commit 374eeeba (from PR #3760). This change introduced the issue.

Initially (in my first commit), I addressed this issue by modifying pointwise_become_partial so that it creates a copy of vmobject.points if vmobject is self, avoiding in-place modification of vmobject.points when self.points is set to np.empty(...). Making a copy is essential because self.points and vmobject.points are used together after self.points is set to np.empty(...). See the related source code.

However, I realized that a simpler solution exists which avoids modifying pointwise_become_partial. Instead, the issue can be resolved by adjusting how Line uses it: When calling it, instead of passing vmobject=self, we should pass vmobject=self.copy(). I’ve implemented this in my most recent commit.

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

[chopan050]

Thanks for the PR! Indeed, when I wrote PR #3760, I missed the case where vmobject was the same Mobject as self (which wasn't an issue before my PR, because .get_cubic_bezier_tuples() returned a copy of the points). Thanks for catching this!

However, I think that your first approach was better:

Initially (in my first commit), I addressed this issue by modifying pointwise_become_partial so that it creates a copy of vmobject.points if vmobject is self, avoiding in-place modification of vmobject.points when self.points is set to np.empty(...). Making a copy is essential because self.points and vmobject.points are used together after self.points is set to np.empty(...).

because of two reasons:

• Mainly, it is a more general fix which addresses the root cause of the problem: I missed to account for when vmobject is self, which seems to be a pretty common case (to make a Mobject become a part of itself). Many other methods use pointwise_become_partial() and we have to account for the case where the template VMobject may also be the transformed VMobject.

• The first approach involves copying only the NumPy array of points (and only in the case where vmobject is actually self), whereas the current approach involves copying the whole VMobject (including points, colors and others), which is more expensive. Although Line._account_for_buff() is a method which is called once and thus it doesn't really matter, it could matter if we implemented this in other methods which involve for loops.

So, could you please use the first approach for this PR? Thanks in advance!

EDIT: Plus, to prevent things like this from breaking again, can you please add some tests as well? At least one for the general case where vmobject is self, and maybe a check that points are calculated correctly for Line when passing both buff and path_arc.

[irvanalhaq9]

Hi @chopan050,
Thanks for the review!

I've restored the fix where vmobject.points is copied when vmobject is self.

Added 2 tests:

• test_pointwise_become_partial_where_vmobject_is_self
• test_line_with_buff_and_path_arc

Let me know if anything else needs adjusting!

[chopan050]

LGTM! Thanks for the changes!