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

VGroup can now be initialized with VMobject iterables #3966

Conversation

NikhilaGurusinghe
Copy link
Contributor

@NikhilaGurusinghe NikhilaGurusinghe commented Oct 20, 2024

Overview

Updates the add method within the VGroup class to allow it to accept iterables, including generators, and, as was the case before this PR, individual VMobject instances or a list of them. Tests have been added for both the Cairo renderer and OpenGL. Hence, implementation is compatible with OpenGL.

Closes #3540

Changelog

  • VGroup can now be initialized with VMobject iterables, such as generators

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

@@ -24,6 +24,7 @@
from manim.constants import *
from manim.mobject.mobject import Mobject
from manim.mobject.opengl.opengl_compatibility import ConvertToOpenGL
from manim.mobject.opengl.opengl_mobject import OpenGLMobject

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
manim.mobject.opengl.opengl_mobject
begins an import cycle.
@NikhilaGurusinghe NikhilaGurusinghe changed the title V group should parse iterables VGroups parse iterables Oct 20, 2024
@NikhilaGurusinghe NikhilaGurusinghe changed the title VGroups parse iterables VGroups can be initialized with iterables Oct 20, 2024
@NikhilaGurusinghe NikhilaGurusinghe changed the title VGroups can be initialized with iterables VGroups can now be initialized with iterables Oct 20, 2024
@NikhilaGurusinghe NikhilaGurusinghe changed the title VGroups can now be initialized with iterables VGroup can now be initialized with iterables Oct 20, 2024
@NikhilaGurusinghe NikhilaGurusinghe changed the title VGroup can now be initialized with iterables VGroup can now be initialized with VMobject iterables Oct 20, 2024
Copy link
Member

@JasonGrace2282 JasonGrace2282 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks pretty good, thanks for the PR!

I left a few comments, but I think the one thing we need to add here is some
documentation. Maybe a small example like below is a good idea to add?

class VGroupParseIterablesExample(Scene):
    def construct(self):
        v = VGroup(
            Square(),
            [Circle(), Triangle()],
            Dot(),
            (Dot() for _ in range(2)),
        )
        v.arrange()
        self.add(v)

manim/mobject/types/vectorized_mobject.py Outdated Show resolved Hide resolved
manim/mobject/types/vectorized_mobject.py Outdated Show resolved Hide resolved
manim/mobject/types/vectorized_mobject.py Outdated Show resolved Hide resolved
manim/mobject/types/vectorized_mobject.py Outdated Show resolved Hide resolved
@NikhilaGurusinghe
Copy link
Contributor Author

NikhilaGurusinghe commented Oct 21, 2024

Overall, looks pretty good, thanks for the PR!

I left a few comments, but I think the one thing we need to add here is some documentation. Maybe a small example like below is a good idea to add?

class VGroupParseIterablesExample(Scene):
    def construct(self):
        v = VGroup(
            Square(),
            [Circle(), Triangle()],
            Dot(),
            (Dot() for _ in range(2)),
        )
        v.arrange()
        self.add(v)

Thanks for the example, I've added it plus some slight explanation to the add function's docstring in VGroup. You can see it here via the read the docs build.

Let me know if there's a better place to put it or if you need me to add more documentation! 😄

Copy link
Member

@JasonGrace2282 JasonGrace2282 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! One minor doc comment, and after that I'll merge this. Thanks for the PR :)

manim/mobject/types/vectorized_mobject.py Show resolved Hide resolved
@JasonGrace2282
Copy link
Member

Thanks :)

@JasonGrace2282 JasonGrace2282 merged commit d5b8b41 into ManimCommunity:main Oct 23, 2024
18 checks passed
@NikhilaGurusinghe
Copy link
Contributor Author

Thanks for all your feedback and help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

VGroup should parse iterables
2 participants