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

Using Swap on two Mobjects in a group can cause self.remove to not work on that group #4210

Open
JedGrabman opened this issue Apr 2, 2025 · 6 comments · May be fixed by #4211
Open

Using Swap on two Mobjects in a group can cause self.remove to not work on that group #4210

JedGrabman opened this issue Apr 2, 2025 · 6 comments · May be fixed by #4211

Comments

@JedGrabman
Copy link

Description of bug / unexpected behavior

After using Swap on two Mobjects in the same Group, I have found that attempts to use self.remove to remove the Group sometimes fail and the Group remains visible

Expected behavior

Running self.remove on a Group should cause that Group to no longer be visible and preceding Swap commands should not affect this.

How to reproduce the issue

Code for reproducing the problem
class SwapTest(Scene):
    def construct(self):
        text_a = Text("A").move_to([1, 0, 0])
        text_b = Text("B").move_to([2, 0, 0])
        text_group = Group(*[text_a, text_b])
        self.play(FadeIn(text_group))
        self.play(Swap(text_a, text_b))
        # Note that uncommenting the following command causes the later self.remove to work as expected
        # self.play(text_group.animate.move_to([0,0,0]))
        self.remove(text_group)
        self.wait(1)

Additional media files

SwapTest.mp4

When uncommenting the specified line, we see self.remove(text_group) working as expected

SwapTest.mp4

System specifications

System Details
  • OS Microsoft Windows 11 Home
  • Python version (python --version) - Python 3.11.4
  • Installed modules (provide output from pip list):
WARNING: Ignoring invalid distribution ~ip (C:\Python311\Lib\site-packages)
WARNING: Ignoring invalid distribution ~umpy (C:\Python311\Lib\site-packages)
Package            Version
------------------ -----------
av                 13.1.0
beautifulsoup4     4.13.3
certifi            2024.12.14
cftime             1.6.4.post1
click              8.1.7
cloudpickle        3.1.0
cloup              3.0.5
colorama           0.4.6
dask               2024.12.1
decorator          5.1.1
fsspec             2024.12.0
glcontext          3.0.0
importlib_metadata 8.5.0
iniconfig          2.0.0
isosurfaces        0.1.2
locket             1.0.0
manim              0.19.0
ManimPango         0.5.0
mapbox_earcut      1.0.2
markdown-it-py     3.0.0
mdurl              0.1.2
moderngl           5.11.1
moderngl-window    3.1.1
multipledispatch   1.0.0
netCDF4            1.7.2
networkx           3.3
numpy              2.2.4
packaging          24.2
pandas             2.2.3
partd              1.4.2
pillow             10.4.0
pip                25.0.1
pluggy             1.5.0
portalocker        3.1.1
pycairo            1.26.1
pydub              0.25.1
pyglet             2.0.17
pyglm              2.8.1
Pygments           2.18.0
pyrr               0.10.3
pytest             8.3.4
python-dateutil    2.9.0.post0
python-sat         1.8.dev14
pytz               2024.2
pywin32            308
PyYAML             6.0.2
rich               13.7.1
scipy              1.14.0
screeninfo         0.8.1
setuptools         78.1.0
six                1.17.0
skia-pathops       0.8.0.post1
soupsieve          2.6
srt                3.5.3
svgelements        1.9.6
toolz              1.0.0
tqdm               4.66.5
typing_extensions  4.12.2
tzdata             2024.2
watchdog           4.0.2
wheel              0.45.1
xarray             2025.1.1
zipp               3.21.0

Additional comments

self.remove(*text_group) (with the unpacking operator *) works as expected

@irvanalhaq9
Copy link
Contributor

The issue with this code is that the Swap animation class accepts individual mobjects,
not text_group, so text_group is not actually added to the scene.
Therefore, removing it will have no effect because it was never in the scene in the first place.
If you want to remove that group, you need to add it to the scene first.

Example:

class SwapTest(Scene):
    def construct(self):
        text_a = Text("A").move_to([1, 0, 0])
        text_b = Text("B").move_to([2, 0, 0])
        text_group = Group(*[text_a, text_b])
        self.play(FadeIn(text_group))
        self.play(Swap(text_a, text_b))
        self.add(text_group)

        self.remove(text_group)
        self.wait(3)

Why does self.remove(text_group) work after self.play(text_group.animate.move_to([0,0,0])) ?
Because self.play adds text_group to the scene.

Another Example where self.remove(text_group) does not work

In the following code, self.remove(text_group) also does not work because the objects passed to FadeIn are individual mobjects.

class SwapTest3(Scene):
    def construct(self):
        text_a = Text("A").move_to([1, 0, 0])
        text_b = Text("B").move_to([2, 0, 0])
        text_group = Group(*[text_a, text_b])
        self.play(FadeIn(text_a, text_b))

        self.remove(text_group)
        self.wait(3)

@JedGrabman
Copy link
Author

Interesting. I'm still a bit confused by the behavior however. The group is displayed on screen after running self.play(FadeIn(text_group)), which I would expect to add the group to the scene. Indeed, if we remove the Swap command entirely, we see that self.remove does cause the group to no longer be displayed on screen, suggesting that it was part of the scene at one point, as in the following example:

class SwapTest4(Scene):
    def construct(self):
        text_a = Text("A").move_to([1, 0, 0])
        text_b = Text("B").move_to([2, 0, 0])
        text_group = Group(*[text_a, text_b])
        self.play(FadeIn(text_group))
        self.wait(1)
        self.remove(text_group)
        self.wait(1)

However, if we include self.play(Swap(text_a, text_b)) (as in the original example) the outcome of running self.remove changes. This suggests something about using Swap caused the group to no longer be recognized as part of the scene (perhaps because individual members were manipulated). This seems like undesired behavior to me, particularly since the members of the group are still visible and can be manipulated by text_group.animate, suggesting they are still part of the group. In general, I'd expect when items become visible on screen due to their membership in a group, they ought to be removable from the screen by removing the group.

I think this also raises a question about how self.remove should act when given a group that is not part of the scene. The behavior seems to differ from how Python lists and sets implement their versions of remove (which throw an error when attempting to remove something that is not present), which may cause confusion for users.

@irvanalhaq9
Copy link
Contributor

irvanalhaq9 commented Apr 2, 2025

This is actually the behavior of Swap. Since Swap, which is essentially just another name for CyclicReplace, operates on individual mobjects, it internally creates a new group for those individual mobjects. This new group is always different from the previously created group. That’s why the original group no longer has any effect unless it is re-added to the scene, either through self.add or implicitly via self.play.

The same applies to FadeIn. If you initially add a group to the scene but later use FadeIn again, but this time on the individual members of that group instead of the group itself, FadeIn will create a new group different from the original one. As a result, removing the original group from the scene will have no effect, because the group currently present in the scene is actually a different one.

class SwapTest4(Scene):
    def construct(self):
        text_a = Text("A").move_to([1, 0, 0])
        text_b = Text("B").move_to([2, 0, 0])
        text_group = Group(*[text_a, text_b])
        self.play(FadeIn(text_group))

        # At some point, you use FadeIn again, but now on the individual mobjects.
        # self.remove(text_group) will have no effect after this.
        self.play(FadeIn(text_a, text_b))

        self.wait(1)
        self.remove(text_group)
        self.wait(1)

Another approach

I'm not sure whether Swap behavior should be considered unexpected behavior or not.
But if it is, there is actually a way to make the original group remain recognized:
by modifying the internal code of CyclicReplace.
This could be done by allowing it to accept an already created group and use that group instead of creating a new one.

However, you would need to pass in the group itself.
If you pass in the individual members of the group instead, the result will still be the same.

@irvanalhaq9 irvanalhaq9 linked a pull request Apr 3, 2025 that will close this issue
3 tasks
@irvanalhaq9
Copy link
Contributor

irvanalhaq9 commented Apr 3, 2025

I have submitted a pull request to address this issue: #4211.

Now, CyclicReplace (which Swap is an alias for) can accept Group or VGroup directly instead of always creating a new group. This ensures that the original group remains in the scene, so it can be removed properly when needed.

Example:

class SwapTest5(Scene):
    def construct(self):
        text_a = Text("A").move_to([1, 0, 0])
        text_b = Text("B").move_to([2, 0, 0])
        text_group = Group(*[text_a, text_b])
        self.play(FadeIn(text_group))

        self.play(Swap(text_group))  # now passing that group
        self.remove(text_group)      # it can be removed

        self.wait(2)

@JedGrabman
Copy link
Author

Great, thanks for the info and PR!

I'm surprised to learn that groups can be removed from a scene as a side effect of so many actions.

I think the practical solution for me will be to use self.remove(*group) instead of self.remove(group) if I want to ensure removal of all group members.

I'm happy to consider the issue closed once the PR goes through.

@irvanalhaq9
Copy link
Contributor

Glad to hear that! Yes, self.remove(*group) is a good workaround.

It seems that when we create a new group with the same members and add it to the scene, the scene automatically replaces the old group with the new one. Here’s an illustration:

from manim import *
text_a = Text("A").move_to(LEFT)
text_b = Text("B").move_to(RIGHT)

# we create a group for those mobjects
group1 = Group(text_a, text_b)

# create a scene
scene = Scene()

scene.add(group1) 
print(scene.mobjects)  # only 1 mobject in the scene
print(group1 is scene.mobjects[0])  # True

# now we create a new group with the same members
group2 = Group(text_a, text_b) 
scene.add(group2)  # add new group to the scene
print(scene.mobjects)  # it remains 1 mobject in the scene
print(group1 is scene.mobjects[0])  # False
print(group2 is scene.mobjects[0])  # True

So, the scene does not store both groups but replaces the previous one.

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 a pull request may close this issue.

2 participants