ManimCommunity / ManimCommunity/manim
Proposal to remove `Mobject.submobjects`, allow only Groups to have `submobjects`, and define `get_submobjects()` to expose submobjects
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 40.9k
- Forks
- 3.1k
- Avg merge
- 3d 12h
- Merged PRs (30d)
- 25
Description
This proposal is related to the issue #4339 and attempts to solve the described problem in a different way:
> ## Description of problem
>
> Sometimes we store child mobjects of some composed mobject as attributes, while also `.add`-ing them to the `self.submobjects` list directly.
>
> This is fine in principle, but has the odd side effect of producing "mismatches" when a user updates the attribute by reassinging it, because the actually for rendering relevant `self.submobjects` list still holds a reference to the former attribute value.
>
> As an example:
>
> ```
> sq = Square()
> b = BraceText(sq, "hello") # this stores the label in `b.label`
> b.label = Text("new value") # and here we reassign the label ...
> ```
>
> This example still renders as a brace with label `"hello"`, because the reference in the `self.submobjects` list is never updated.
## Proposed solution
The idea is the same: to have a single reference to each submobject, a single source of truth for them.
It does not make a lot of sense that "basic" Mobjects such as `Circle`, `Square`, `ImageMobject` or `ValueTracker` have submobjects. Instead, it makes more sense to take a `Circle` and group it inside a `Group` or `VGroup` containing other Mobjects. This leads me to the idea that maybe only Groups should have `submobjects` as such.
Therefore, my idea is:
- The base class `Mobject` **does not** have `.submobjects`. It also **does not** have methods such as `.add()`, `.remove()`, etc.
- Only `Group`, `VGroup` and theirs subclasses have `.submobjects`, `.add()`, `.remove()`, etc.
- Groups should be a special Mobject which is more clearly separated from the rest of Mobjects. For example, currently `Axes` is a subclass of `VGroup` and `CoordinateSystem`. It **should not** be a subclass of `VGroup`. `Axes` should have another way of exposing its submobjects and should not have `.submobjects`, `.add()`, `.remove()`, etc. See a more detailed description of how to achieve that below.
In this way, if only Groups have submobjects and you can only directly add submobjects to Groups, then we put a stricter limit to the unexpected behaviors which might arise from, say, adding a Mobject which already exists.
We still need Mobjects that contain other Mobjects without necessarily being Groups, like `MathTex` and `Axes`. Therefore, I propose defining a `Mobject.get_submobjects()` or `Mobject.get_children()` method.
- For the base class `Mobject`, this should simply return `[]`.
- Other Mobjects which need children may override it. For example, `Axes.get_children()` would return `[self.x_axis, self.y_axis]`. `NumberLine.get_children()` would return something like `[*super().get_children(), *self.ticks, *self.numbers]`.
- Of course, `Circle` does not override the method, so it still returns `[]`.
- `Group.get_children()` and `VGroup.get_children()` simply return `self.submobjects`.
In this way, by getting rid of `.submobjects` where necessary and making `.get_submobjects()` or `.get_children()` return the already existing references to those submobjects, we ensure that there's effectively one single reference for each one of them, solving the described problem.
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
Review the Mobject, Group, VGroup, Axes, MathTex, and NumberLine implementations mentioned in the proposal, beginning with how child objects are currently exposed and added. Treat the work as complete only when the proposed single-reference model and get_submobjects()/get_children() behavior are defined consistently across those classes; no specific test or file is named.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- computer-graphics
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100