ManimCommunity / ManimCommunity/manim
Mobject groups should be improved
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 40.9k
- Forks
- 3.1k
- Avg merge
- 3d 12h
- Merged PRs (30d)
- 25
Description
This was discussed with @WampyCakes and @cobordism on discord.
The current structure of mobject group types (`Group`, `VGroup`, etc.) requires code duplication and knowledge of when to use `VGroup` over (the more idiomatic I'd argue) `Group`.
This could be solved by generating group types like `VGroup = GroupType("VGroup", VMobject)`, which is similar in concept to [`collections.namedtuple`](https://docs.python.org/3/library/collections.html#collections.namedtuple), and then changing `Group` to be a function that returns an instance of a more specific group type depending on the passed mobjects, so for instance `Group(VMobject())` would return a `VGroup`, `Group(Mobject())` would return an `MGroup` (which would be the new name of the generic `Mobject` group type), and `Group(VMobject(), Mobject())` would return an `MGroup`.
If this is done, it should be a non-breaking change, as you could still call `VGroup` and use it, and calling `Group` directly shouldn't affect any functionality.
Here's a proof of concept that demonstrates the functionality:
```py
import inspect
class Mobject:
def add(self, *mobjects):
print(type(self).__name__)
class VMobject(Mobject):
pass
_group_types = {}
def GroupType(name, mob_type):
if mob_type in _group_types:
return _group_types[mob_type]
class _Group(mob_type):
mobject_type = mob_type
def __init__(self, *mobjects):
self.add(*mobjects)
# Get the module name GroupType was called from
# so the generated class's module is not always the
# same module that GroupType is defined in.
last_module = inspect.currentframe().f_back.f_globals["__name__"]
group_type = type(name, (_Group,), dict(
__module__ = last_module,
))
_group_types[mob_type] = group_type
return group_type
MGroup = GroupType("MGroup", Mobject)
VGroup = GroupType("VGroup", VMobject)
def Group(*mobjects, **kwargs):
if len(mobjects) == 0:
return MGroup(**kwargs)
current_group = MGroup
for mobject_type, group_type in _group_types.items():
if (
issubclass(mobject_type, current_group.mobject_type) and
all(isinstance(mob, mobject_type) for mob in mobjects)
):
current_group = group_type
return current_group(*mobjects, **kwargs)
# Returns an MGroup
Group(Mobject())
# Returns a VGroup
Group(VMobject())
# Returns an MGroup
Group(VMobject(), Mobject())
# Returns an MGroup
Group()
```
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the existing Group, VGroup, and generic Mobject group definitions, then compare their structure with the proof of concept in the issue. Check how group construction and type selection currently work. Done means the proposed generated group types and Group dispatch are non-breaking for existing VGroup usage and direct Group calls.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- computer-graphics
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100