Repository: ManimCommunity/manim
Pull request: Check for add in VGroup
<!--- 
Thanks for contributing to manim!
**Please ensure that your pull request works with the latest version of manim from this repository.**
You should also include:
  1. The motivation for making this change (or link the relevant issues)
  2. How you tested the new behavior (e.g. a minimal working example, before/after
     screenshots, gifs, commands, etc.) This is rather informal at the moment, but
     the goal is to show us how you know the pull request works as intended.
If you don't need any of the optional sections, feel free to delete them to prevent clutter.
-->

Solves #284 as suggested in the issue by checking if every submobject is an instance of `VMobject` in the `add` function instead of doing it only at initialization.

## Acknowledgement
- [X] I have read the [Contributing Guidelines](https://github.com/ManimCommunity/manim/wiki/Documentation-guidelines-(WIP))

<!-- Once again, thanks for helping out by contributing to manim! -->


Problem statement:
VGroup only checks input when creating, not when adding
This is good: 
```python
>>> from manim import VGroup, Mobject
>>> g = VGroup(Mobject())
Exception: All submobjects must be of type VMobject
```

This is bad:
```python
>>> g = VGroup()
>>> g.add(Mobject())
<manim.mobject.types.vectorized_mobject.VGroup at 0x7f907b8991d0>
```

VGroup should override `add` to perform input checking.

Hints from issue discussion:

Commit message: !fixup black vectorized_mobject

Patch to review:
diff --git a/manim/mobject/types/vectorized_mobject.py b/manim/mobject/types/vectorized_mobject.py
index 805c230009..c7129aab73 100644
--- a/manim/mobject/types/vectorized_mobject.py
+++ b/manim/mobject/types/vectorized_mobject.py
@@ -846,11 +846,30 @@ def get_subcurve(self, a, b):
 
 class VGroup(VMobject):
     def __init__(self, *vmobjects, **kwargs):
-        if not all([isinstance(m, VMobject) for m in vmobjects]):
-            raise Exception("All submobjects must be of type VMobject")
         VMobject.__init__(self, **kwargs)
         self.add(*vmobjects)
 
+    def add(self, *vmobjects):
+        """Checks if all passed elements are an instance of VMobject and then add them to submobjects
+
+        Parameters
+        ----------
+        vmobjects :
+            List of VMobject to add
+
+        Returns
+        -------
+        None
+
+        Raises
+        ------
+        Exception
+            If one element of the list is not an instance of VMobject
+        """
+        if not all([isinstance(m, VMobject) for m in vmobjects]):
+            raise Exception("All submobjects must be of type VMobject")
+        super().add(*vmobjects)
+
 
 class VDict(VMobject):
     """A VGroup-like class, also offering submobject access by


Code review comments:
diff_hunk: @@ -846,11 +846,30 @@ def get_subcurve(self, a, b):
 
 class VGroup(VMobject):
     def __init__(self, *vmobjects, **kwargs):
-        if not all([isinstance(m, VMobject) for m in vmobjects]):
-            raise Exception("All submobjects must be of type VMobject")
         VMobject.__init__(self, **kwargs)
         self.add(*vmobjects)
 
+    def add(self, *vmobjects):
+        """Checks if all passed elements are an instance of VMobject and then add them to submobjects
+
+        Parameters
+        ----------
+        vmobjects :
+            List of VMobject to add
+
+        Returns
+        -------
+        None
+
+        Raises
+        ------
+        Exception
+            If one element of the list is not an instance of VMobject
+        """
+        if not all([isinstance(m, VMobject) for m in vmobjects]):
path: manim/mobject/types/vectorized_mobject.py
text: @user1:
Suggestion: change `all([isinstance(m, VMobject) for m in vmobjects])` to `all(isinstance(m, VMobject) for m in vmobjects)`

diff_hunk: @@ -846,11 +846,30 @@ def get_subcurve(self, a, b):
 
 class VGroup(VMobject):
     def __init__(self, *vmobjects, **kwargs):
-        if not all([isinstance(m, VMobject) for m in vmobjects]):
-            raise Exception("All submobjects must be of type VMobject")
         VMobject.__init__(self, **kwargs)
         self.add(*vmobjects)
 
+    def add(self, *vmobjects):
+        """Checks if all passed elements are an instance of VMobject and then add them to submobjects
+
+        Parameters
+        ----------
+        vmobjects :
+            List of VMobject to add
+
+        Returns
+        -------
+        None
+
+        Raises
+        ------
+        Exception
+            If one element of the list is not an instance of VMobject
+        """
+        if not all([isinstance(m, VMobject) for m in vmobjects]):
+            raise Exception("All submobjects must be of type VMobject")
path: manim/mobject/types/vectorized_mobject.py
text: @user1:
Suggestion: while we're at it, we could change this `Exception` to a `TypeError`. See [here](https://docs.python.org/3/library/exceptions.html#TypeError).

EDIT: if you do change it to a TypeError, please add a `Raises` section in the docstring.

diff_hunk: @@ -846,11 +846,30 @@ def get_subcurve(self, a, b):
 
 class VGroup(VMobject):
     def __init__(self, *vmobjects, **kwargs):
-        if not all([isinstance(m, VMobject) for m in vmobjects]):
-            raise Exception("All submobjects must be of type VMobject")
         VMobject.__init__(self, **kwargs)
         self.add(*vmobjects)
 
+    def add(self, *vmobjects):
+        """Checks if all passed elements are an instance of VMobject and then add them to submobjects
+
+        Parameters
+        ----------
+        vmobjects :
path: manim/mobject/types/vectorized_mobject.py
text: @user1:
gotta tell us their type
```suggestion
        vmobjects : :class:`~.VMobject`
```

@author:
By the way is there a particular way to treat star arguments in the docstring or is it fine this way ?

@user1:
@author refer to the documentation guidelines for this, but: you document the type of the expected parameters.