ManimCommunity / ManimCommunity/manim

Some ideas for specifying mobject defaults and inheritance

Open
#1,023 3 comments 1 reaction 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
40.9k
Forks
3.1k
Avg merge
3d 12h
Merged PRs (30d)
25

Description

## Motivation

In the past week or so I've been noticing quite a lot of weird things regarding manim object properties. Bug #1020 is one example, but in the same vein @friedkeenan pointed out on Discord that ScreenRectangle doesn't work as expected because `Rectangle.__init__` overrides some important values.

Overall, it seems like the trouble might stem from either the CONFIG rewrite (#783 - no blame, CONFIG needed to go) or just the general messiness of inheritance (in both the class hierarchy sense and the scene hierarchy sense).

I'd like to collect my thoughts on inheritance and defaults here, and encourage conversation on what makes sense going forward. I'm still pretty new to the repo, so I would really value other's thoughts.

Ultimately, the goal is to set up the inheritance system under the hood so it's a lot harder to introduce bugs like the two mentioned above.

## Vision

It's important to start by naming what the default mobject property system is for, to keep the goals in mind. Two of these are done pretty well at the moment, one is not.
- smart defaults for mobjects, so that animations can be made quickly and iteratively
- set properties for a specific object, because - well, duh.
- set default for a class of objects so that animators can quickly and concisely create an aesthetic for their own videos.

From previous iterations, there are a couple important lessons to keep in mind when we're modifying the new system.
- Minimal changes to [reasonable] current behavior.
- Explicit is better than implicit (as one of Python's maxims). One of the problems with CONFIG (as I understand it - refering to @leotrs on this one) was that the arguments weren't explicit and so couldn't be parsed by IDEs.
- Priority should be given to the more specific set of properties (this is like CSS).

#### Which defaults take priority?

One thing to note is that this would be straightforward if there was only one type of inheritance, but there are two - the classes and the scene graph. For example, a `Cross` is a subclass of `VGroup` but its objects are `Line`s.

Ultimately, though, I have something I think makes sense. From highest to lowest priority:

1) Properties set imperatively, e.g, `thing.set_color(RED)` (or `thing.color=RED` after #787 is completed).
2) Properties set in the scripter-facing kwargs of a Mobject or a Group, like `Line(color=BLUE)` or `Cross(obj, stroke_width=24)`.
3) Properties set in the file the object is loaded from (really only applying to SVG files here).
4) Property class defaults set imperatively, e.g, `Square.color = RED` at the top of a scene file. Highest priority is the current class, with decreasing priority up the class hierarchy.
5) If we allow `.cfg` files to specify class-level properties, then they would have priority here. Again, more specific classes take priority over more general classes.
6) Hard-coded defaults within the class's source code, aka the community-wide default, and on up the class hierarchy.

## Implementation

For implementation, I have some initial thoughts. Not all of these are exclusive.

- To enable class defaults, it might make sense to have class properties. I did a smidge of testing and it looks like inheritance works the way you'd expect with class properties, as long as the class properties aren't mutable. I'm not sure whether it's pythonic, but I would hope someone knows that well enough here (@leotrs?)
- I think it would make a lot of sense to have color, stroke, and other defaults for classes enabled within the config files. When I first learned about config files I thought that's what they were for, and I was slightly disappointed that's not how it worked.
- A final stray thought is to consider having the property passing as `**kwargs` within the init function signature, but explicitly listing off all the properties either as a class property or within the `__init__` call. It might make merging of the property dictionaries a lot easier, while also keeping the properties visible to the IDE (and therefore the user)

Some tricky cases came to mind:

- How should a Square that is a submobject of a Line be colored - by its Square [class] default or its scene-hierarchy-parent Line coloring? I think it should be its square coloring, and there should be a test case for that in particular.
- Is `None` ever used as a meaningful parameter, rather than the lack of a parameter? If so, that might make it really difficult to specify explicit nothing-was-set-for-this-parameter as `None`.
- What if a user wants to specify all rectangles a certain color, but leave the Riemann rectangles in `GraphScene` their default color? It might be possible to create a stubbed subclass of Rectangle and specify its default that way, but another option is specifying that rectangles under a RiemannRectangle group shouldn't be colored...
- What to do about double class inheritance (do we have any objects of that type?)
- Some objects set their submobjects's colors imperatively within the class, e.g. GraphScene. Where does that rest on the hierarchy? I think (1) and (2) obviously apply, but should it be influenced by (4) like `Rectangle.color=RED` or (5) in the config file? And what if there was a specified default, in, say, a config file entry (5) for GraphScene's `default_start_color`? Would that trump the less specific but higher priority class-default-in-file (4) for rectangle color?

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the inheritance and default-property behavior described for Mobject, Rectangle, ScreenRectangle, Cross, Line, and GraphScene, along with issues #1020, #783, and #787. Because this is an open-ended design discussion with no named files or tests, first establish the intended priority rules and define test cases for the listed tricky cases; done requires an agreed design and implementation scope.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.