Some ideas for restructuring Mobject attributes.

[kolibril13]

As we are restructuring how manim is configured right now, I have some suggestions:

class Example(Scene):
    def construct(self):
        dot = Dot().set_color(GREEN)
        dot.set_color(RED)
        dot.color=BLUE
        print(dot.color)
        self.add(dot)
        self.wait(1)

This script adds a red dot to the scene, however, it prints the # for blue color.
Here it would be nice if dot.color=BLUE would actually change the color to blue on the displayed dot.

Secondly:

class Example(Scene):
    def construct(self):
        dot = Dot(radius=0.1)
        print(dot.radius) # prints 0.1
        dot.set_width(2)
        print(dot.radius) # prints 0.1
        dot.radius = 10
        print(dot.radius) # prints 10
        self.add(dot) # adds dot with radius 2

Here, we can set the width of a dot, which changes its radius, but the attribute parameter of Dot is not touched.
At the same time, when the radius attribute of Dot is touched, the displayed dot won't change its shape.
I think that this is not a big problem, but in case there is a nice solution to it, that would be great!

And thirdly:

class Example(Scene):
    def construct(self):

        t1 = Text("1")
        t2 = Text("2").next_to(t1)
        t3 = Text("3").next_to(t2)

        self.add(t1,t2,t3)

This is producing White text for all three text Mobjecs. When a white background is used, I think it would be nice to have an easy option to change the default parameter once of Text at the beginning of the script, without the necessity to change it for all three TextMobjects

[leotrs]

1. Does your code still not work as intended after Fix 258: set_color(COLOR) doesn't change the self.color attribute #751 was merged?

2. I agree it should be possible to change the general size/shape/geometry of a Mobject by setting some of its attributes. But this also brings up the question of animating methods. For example, it's nice to be able to do self.play(mobject.set_radius, 0.2) to animate the change of size. If we implement a refactor that gets rid of all set_* methods and instead encourages people to do self.radius = 0.2, then this is no longer possible. However, in the interest of getting rid of set_* methods, I think we should implement a single Mobject.set() method that can be passed to play. This was discussed recently with @friedkeenan on discord.

3. I agree we should have something like config.default_mobject_color that overrides the initial color of all Mobjects.

[kolibril13]

1. Does your code still not work as intended after Fix 258: set_color(COLOR) doesn't change the self.color attribute #751 was merged?

I just ran it on the current master and it does not work. I think #751 only fixed that dot.set_color(RED) will change the attribute to red as well, but dot.color=BLUE changes only the attribute, not the color of the displayed dot.

2. I agree it should be possible to change the general size/shape/geometry of a Mobject by setting some of its attributes. But this also brings up the question of animating methods. For example, it's nice to be able to do self.play(mobject.set_radius, 0.2) to animate the change of size. If we implement a refactor that gets rid of all set_* methods and instead encourages people to do self.radius = 0.2, then this is no longer possible. However, in the interest of getting rid of set_* methods, I think we should implement a single Mobject.set() method that can be passed to play. This was discussed recently with @friedkeenan on discord.
   Another option would be that both are possible:

• self.radius = 0.2 in normal code to setup a scene
• self.play(mobject.set_radius, 0.2) when it is used in an animation code
  But I also really like the idea of having a Mobject.set() method.

3. I agree we should have something like config.default_mobject_color that overrides the initial color of all Mobjects.

Indeed, config.default_mobject_color would be good! But I think additionally it would be great if the initial colour could be also set for only one Mobject (like e.g. the Text).

[behackl]

Would it also be possible to work with a static variable here? Like, setting Text.color (or Text.default_color?) to make subsequent calls to Text use the corresponding default color?

[leotrs]

I like that idea a lot! The pro when using the config system is that you can set it once and for all in a user-code config file for example. The con is that it is not as flexible as doing it for each specific class, like you are suggesting.

…

On Mon, Nov 30, 2020, 4:02 AM Benjamin Hackl ***@***.***> wrote: Would it also be possible to work with a static variable here? Like, setting Text.color (or Text.default_color?) to make subsequent calls to Text use the corresponding default color? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#787 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAILYAHT2CZDBVRQXKXZW3TSSNNSFANCNFSM4UGTFUUQ> .

[XorUnison]

As @leotrs has mentioned in response to a question I posted on discord, what about using properties?
Using properties should allow us to get the best of all worlds, that is initializing Class(x=y) and changing objects Class.x=y correctly, getting correct values at all times with y=Class.x, while also not having anything like Class.set_x or Class.get_x publicly accessible nor needed. One way to access and set variables that always works. Of course this would mean rewriting tons of code to do this everywhere.

[leotrs]

Yes! I sort of wrote all of the above implicitly assuming that color and similar attributes would be properties 😅

[friedkeenan]

@leotrs and I discussed a potential solution for this on discord a few days ago, so I'll try to condense what we ended up coming up with here. Here though is essentially the conclusion of that conversation on discord.

So basically, all mobject.get_* and mobject.set_* methods would be turned into properties, so get_color and set_color would be turned into a singular color property. This would allow you to do things like obj1.color = obj2.color, and have everything work out as expected, setting obj1's color to obj2's.

The problem with that though is animating setting those properties, because normally you would do something like self.play(obj1.set_color, obj2.get_color()) and that would call the method with the arguments to get two objects to interpolate between. And since you can't do self.play(obj1.color = obj2.color), we need a way to do something similar to that, and which stays mostly consistent with how you'd get and set properties outside of animations.

While we're on the topic of consistency, it doesn't seem especially consistent that when calling a mobject's method outside of an animation, you do it like obj.method(arg), but then when animating it you do self.play(obj.method, arg). They're pretty close, but what if we could get closer? What @leotrs and I came up with at the time was to give every mobject an animate attribute (or anim attribute if animate is deemed too long), which would be of a special class which overrides __getattr__ to do some machinery to allow self.play(obj.animate.method(arg)) instead of self.play(obj.method, arg). The class for the animate attribute could look something like this:

class AnimateUtil:
    def __init__(self, mobject):
        self.mobject = mobject

    def __getattr__(self, attr):
        def real_animate(*args, **kwargs):
            method = getattr(self.mobject, attr)

            return animation_from_method(method, *args, **kwargs)

        return real_animate

This would bring more consistency between how methods are run outside and inside of animations, at least user-facing consistency. This would also greatly improve passing keyword arguments to methods when animating them, since you could do self.play(obj.animate.method(kwarg=blah)) instead of self.play(obj.method, {"kwarg": blah}).

The next part of the puzzle would be a universal set method for mobjects, something like this:

def set(self, **kwargs):
    for attr, value in kwargs.items():
        setattr(self, attr, value)

    return self

Then you could do things like obj.set(color=BLUE), which has the benefit of showing color=BLUE, which is similar to obj.color = BLUE. The main benefit though is that you could then do self.play(obj.animate.set(color=BLUE)), which looks very readable to me, and which also would allow for setting multiple attributes at once in an animation with no extra hassle.

With all of this together, these two would be equivalent ways of changing the color attribute:

obj.color = BLUE
obj.set(color=BLUE)

And these two would be equivalent ways of animating the change of color:

self.play(obj.animate.set(color=BLUE))
self.play(obj.set, {"color": BLUE})

And as @leotrs said on discord, perhaps the fourth one could be deprecated in favor of the third at some point.

Personally, I'm a fan of this approach and I think it gives a really pleasing API for users.

[leotrs]

Just two technical points on the above:

1. obj.animate.attr(args) returns an instance of Animation. With a little more work, we could allow users to define their own custom methods that return their own custom animations. Note this makes Scene.play's job easier since it no longer has to just stick an ApplyMethod in front of everything.

2. I am on the fence about deprecating the fourth syntax in the previous comment, perhaps it can still be useful in case some animations are being generated programmatically. But only time will tell!

[cobordism]

Now that #783 has been merged, we can set about restructuring mobject attributes more generally.

I'd be willing to try and implement something along the lines of #787 (comment) but I can't do it alone. I'll need help.

Am I right in assuming that the 'correct' implementation of these getter/setter methods requires some form of @-decorators?

[leotrs]

Yes! Basically, the following code:

def get_val(self):
    return self.val
def set_val(self, new_val):
    self.val = new_val

is Bad Python, and could/should be replaced with:

@property
def val(self):
    return the_value_of_val
@val.setter
def val(self, val):
    # do something to change the value of val

In the easiest case, the property is just an interface to an underlying attribute.

class MyCircle:
    def __init__(self, radius):
        self.radius = radius
    @property
    def diameter(self):
        return self.radius * 2
    @diameter.setter
    def diameter(self, val):
        self.radius = val / 2

So now you can do both self.diameter = 2 and both the self.radius and self.diameter attributes will reflect that.

This is exactly the intended use-case of properties. I think that a good first pass is to try to deprecate getters and setters and replace them with properties all over the codebase, as much as possible.

[leotrs]

Perhaps @eulertour or @PgBiel want to chime in. This is the moment to retake our discussion about dataclasess and attrs, perhaps?