Add generic set method and compatibility layer between properties and get_*/set_* methods

[friedkeenan]

Motivation

A generic set method is useful for animating setting properties, as discussed in #787. The compatibility layer added also automatically maintains backwards compatibility with the get_* and set_* API for any added properties, which should leave code for added properties cleaner.

Overview / Explanation for Changes

The set method allows mobject attributes to be defined as properties instead of with get_* and set_* methods while allowing setting the properties to be animated with the .animate syntax, which is more pythonic and leads to less unintuitive code, like how obj.color = BLUE doesn't truly set the mobject's color to blue.

The automatic compatibility layer allows you to create a mobject like so:

class MyMobject(Mobject):
    @property
    def blah(self):
        return self._blah

    @blah.setter
    def blah(self, value):
        self._blah = value

and still be able to use get_blah and set_blah without having to create a mobject like so:

class MyMobject(Mobject):
    def get_blah(self):
        return self._blah

    def set_blah(self, value):
        self._blah  = value
        return self
    
    blah = property(get_blah, set_blah)

the last line of which seems particularly unpythonic/unconventional/less readable to me.

The compatibility layer is useful for at least as long as we're in the process of transitioning to using properties for mobject attributes, and then once that transition is complete we can decide whether we want to keep the compatibility layer and just deprecate the corresponding get_* and set_* methods, or if we want to remove them outright with a breaking change. The layer also does not interfere with get_* and set_* methods that are defined normally.

I've also added unit tests to ensure all this works.

Oneline Summary of Changes

- Added generic set method and compatibility layer between properties and get_*/set_* methods (:pr:`995`)

Testing Status

All tests pass locally.

Further Comments

Currently unsure of where/how to document the compatibility layer, or if it needs to be documented at all.

Acknowledgements

• I have read the Contributing Guidelines

Reviewer Checklist

• Newly added functions/classes are either private or have a docstring
• Newly added functions/classes have tests added and (optional) examples in the docs
• Newly added documentation builds, looks correctly formatted, and adds no additional build warnings
• The oneline summary has been included in the wiki

[friedkeenan]

Oh, doctest is failing because mob.set(blah=0) returns mob but I have it not returning anything. Unsure of how to handle that.

EDIT: Nevermind I just need to make it expect Mobject

[ponmike92]

@friedkeenan really cool feature! Look forward for it. Is it half way of cutting off get_stroke_width(), get_stroke_opacity() and and so on and replacing them with stroke_width, stroke_opacity ... properties?

[friedkeenan]

@friedkeenan really cool feature! Look forward for it. Is it half way of cutting off get_stroke_width(), get_stroke_opacity() and and so on and replacing them with stroke_width, stroke_opacity ... properties?

Yeah, the details of what will and won't be properties still needs to be hammered out on a case by case basis, but the plan is to migrate to a more property-driven API, and this PR is a step towards that

[friedkeenan]

Okay, added documentation for the compatibility layer along with a warning saying it's not the preferred way of setting/getting attributes and that the layer could potentially go away in the future.

[huguesdevimeux]

I have trouble understanding this.
Are you trying to build a compatlibity layer, so we can progressively replace everything with @Property while preventing other old stuff still using set_ and get_ from breaking?
If yes, then i'm totally in favor, but I think this should absolutely have a DepreciationWarning. Therefore, I think this feature should come when the work in replacing set_ and get_ by @Property will be started.

[friedkeenan]

Are you trying to build a compatlibity layer, so we can progressively replace everything with @Property while preventing other old stuff still using set_ and get_ from breaking?

Yes

I think this should absolutely have a DepreciationWarning.

Sounds good to me, will probably add that later then.

Therefore, I think this feature should come when the work in replacing set_ and get_ by @Property will be started.

Well I was hoping for this to be merged so that the migration to properties can start, I was thinking incremental PRs could be made covering specific mobject attributes instead of one big go. Merging this would also allow users creating their own Mobject subclasses to start using a more property-driven API right away.

[huguesdevimeux]

Well I was hoping for this to be merged so that the migration to properties can start, I was thinking incremental PRs could be made covering specific mobject attributes instead of one big go. Merging this would also allow users creating their own Mobject subclasses to start using a more property-driven API right away.

I also think that, but if you plan to do a PR starting the work right after this one then it's ok. I just don't want that this gets forgotten.

And, for the DepreciationWarning, it'd make more sense to add it only for properties that have been changed not for all in one short. Not sure if it's possible, though.

[friedkeenan]

I also think that, but if you plan to do a PR starting the work right after this one then it's ok. I just don't want that this gets forgotten.

Personally I'm kinda itching to start migrating, I don't see this as a big issue.

And, for the DepreciationWarning, it'd make more sense to add it only for properties that have been changed not for all in one short. Not sure if it's possible, though.

Currently the compatibility layer only adds get_*/set_* methods that aren't explicitly defined. So stuff like our current set_color and such aren't affected at all, but if we were to get a color property and remove the getters/setters for colors, then the compatibility layer would affect it. So basically I can just add the warning to the methods the compatibility layer creates

[huguesdevimeux]

I see, then. Thanks for the clarification!

[friedkeenan]

Added DeprecationWarnings to the compatibility layer. Also had to make it so the warnings actually show because manim scripts aren't run with "__main__" as their __name__

[behackl]

Thanks for your efforts, and also thanks for the thorough documentation! LGTM.