Open markromanmiller opened 3 years ago
Hi,
Some trow-on-paper thoughts about this.
To conclude I don't think implementing such thing is a good idea in our situation. As you mentioned, there will be logical issues and will probably get use closer from the old world with CONFIG that getting us far from this.
Thanks for this :D
Thanks for the feedback. So most of the specific implementation ideas don't seem to make sense one way or another. That's good to know.
What do you think about the goals, though?
To perhaps talk about the motivation in a different way: I think it's error-prone to do this property-passing and property-setting the way we have been doing it. I don't seem to notice any rhyme or reason for which properties are attributes and which are not, which are explicit and which are in **kwargs
, when to overwrite properties and when to not, etc etc.
I'm not sure to follow you. Could you show an example ?
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.
From previous iterations, there are a couple important lessons to keep in mind when we're modifying the new system.
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 ofVGroup
but its objects areLine
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)
(orthing.color=RED
after #787 is completed). 2) Properties set in the scripter-facing kwargs of a Mobject or a Group, likeLine(color=BLUE)
orCross(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.
**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:
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 asNone
.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...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'sdefault_start_color
? Would that trump the less specific but higher priority class-default-in-file (4) for rectangle color?