Implementing an official and stable deprecation system

[RickyC0626]

Description of proposed feature

Our current way of deprecating functions and classes could be improved to be more user and developer friendly. Perhaps we can implement a decorator-based system where the developer can input the following information:

• Version since deprecation
• Version until deletion
• Reason for deprecation

How can the new feature be used?

@deprecated(since="0.5.0". until="0.7.0", message="Use Create instead.")
class ShowCreation(Create):
    ...
    
class Animation(object):
    @deprecated(since="0.1.0", until="0.3.0", message="Use animation.interpolate() instead.")
    def update(self, alpha):
        ...

Additional comments

We could also implement helper methods and/or CLI commands that can:

• List deprecated classes/methods
• Sort classes/methods by version since deprecation
• Warn developers if there is any deprecated code that has not been deleted at the expected version
  • i.e. If until="0.6.0" and current version is 0.6.0 or later, a warning will be shown as a reminder to delete the deprecated code

We could deprecate parameters this way as well.

[WampyCakes]

Sounds great to me. Perhaps message would fit more than reason

[RickyC0626]

Sounds great to me. Perhaps message would fit more than reason

I think reason would make more sense semantically as it would prompt developers to give a reason for deprecation, rather than a potentially vague message.

[WampyCakes]

That is a valid point, but if I were reading the decorator as an outsider, I would be confused as to why the message it prints to users is called "reason"

[RickyC0626]

That is a valid point, but if I were reading the decorator as an outsider, I would be confused as to why the message it prints to users is called "reason"

Good point. Deprecation and code removal are simple enough tasks to be considered good first issues. We can go with message and explain in documentation that it should explain the reason for deprecation and/or state the alternative class/method/parameter to use.

[AntonBallmaier]

I am working on an implementation :)

[AntonBallmaier]

tl;dr Until today I didn't know I love annotations... Now I do

I expanded this Idea a bit, to make the common use cases as simple as possible:

@deprecated can be used with or without parameters to deprecate functions, methods and classes.
The following examples should give you an idea of what each parameter does:

@deprecated
@deprecated(since="0.2")
@deprecated(until="0.5")
@deprecated(since="0.2", until="0.5")
@deprecated(replacement="bar", message="It's better.")
def foo():
    pass

calling foo() results in the following logger warnings:

foo()
#Logger warnings:
#  The function foo is deprecated and may be deleted soon. 
#  The function foo is deprecated since 0.2 and may be deleted soon. 
#  The function foo is deprecated and will be deleted after 0.5. 
#  The function foo is deprecated since 0.2 and will be deleted after 0.5. 
#  The function foo is deprecated and may be deleted soon. Use bar instead. It's better.

Name and type of the callable are automatically added to the message:

@deprecated
class Foo:
    @deprecated
    def bar(self):
        pass

a = Foo()
#  The class Foo is deprecated and may be deleted soon. 

a.bar()
#  The method Foo.bar is deprecated and may be deleted soon.

I also implemented deprecated_params.

@deprecated_params(params="useless_param, other_useless_param", until="0.5", message="It really was useless.")
#Equivalent options:
#  @deprecated_params(params="useless_param other_useless_param", ...)
#  @deprecated_params(params=["useless_pram", "other_useless_param"], ...)
def foo(useless_pram=None, other_useless_param=None):
    pass

foo()
# No warning

foo(useless_param=2)
#  The parameter useless_param of function foo is deprecated and will be deleted after 0.5. It really was useless.

foo(useless_param=2, other_useless_param=0)
#  The parameters useless_param and other_useless_param of function foo are deprecated and will be deleted after 0.5. It really was useless. 

However, I think in most cases where we would want to deprecate parameters is because we renamed / grouped / restructured them. Therefor deprecated_params has one additional parameter: redirections. This can be used to automatically redirect parameter values given in an old format, so that the logic to handle those old parameters can already be removed from inside the function. In the end actually deleting the old parameter comes down to just deleting the parameter. Have a look:

@deprecated_params(redirections=[
        ("n_rows", "rows"),
        lambda n_cols: {"cols": n_cols}, # equivalent to ("n_cols", "cols"),
        lambda buff_x=0.1, buff_y=0.1: {"buff": (buff_x, buff_y)}
])
def baz(rows=None, cols=None, buff=None):
    print(f"rows={rows}, cols={cols}, buff={buff}")

baz(n_rows=2)
#  The parameter n_rows of function baz is deprecated and may be deleted soon. 
#  rows=2, cols=None, buff=None

baz(n_cols=2, n_rows=4)
#  The parameters n_rows and n_cols of function baz are deprecated and may be deleted soon. 
#  rows=4, cols=2, buff=None

baz(buff_x=1)
#  The parameter buff_x of function baz is deprecated and may be deleted soon. 
#  rows=None, cols=None, buff=(1, 0.1)

baz(buff_x=1, buff_y=2)
#  The parameters buff_x and buff_y of function baz are deprecated and may be deleted soon. 
#  rows=None, cols=None, buff=(1, 2)



@deprecated_params(redirections=[
        lambda buff: {"buff_x": buff[0], "buff_y": buff[1]} if isinstance(buff, tuple)
                else {"buff_x": buff, "buff_y": buff}
])
def zab(buff_x, buff_y):
    print(f"buff_x={buff_x}, buff_y={buff_y}")

zab(buff=1)
#  The parameter buff of function zab is deprecated and may be deleted soon. 
#  buff_x=1, buff_y=1

zab(buff=(2,3))
#  The parameter buff of function zab is deprecated and may be deleted soon. 
#  buff_x=2, buff_y=3

Notice that you don't have to specify params if you are using redirect. Every redirected parameter is automatically added to the list of deprecated parameters. For redirectors with multiple parameters you should always give default values, since they get called if at least one of the redirected parameters is defined.

Let me know what you think about this. Are the any other useful features the deprecation system should have? Or are there other decorators manim could make good use of? 🙃

[RickyC0626]

@AntonBallmaier This is pretty neat! I must say your idea is much more fleshed out and exceeds expectations.

[AntonBallmaier]

Thanks :) However, I just noticed a problem: Decorating a class will change its type to a function. This doesn't change how it behaves in code, but it will no longer show up in the module classes overview page of the documentation... Hopefully I can fix this tomorrow 😴

[AntonBallmaier]

...now that I think about it: It wouldn't even be that bad if deprecated classes wouldn't be shown in the docs 😆

[AntonBallmaier]

Just double checked: This doesn't cause problems when deprecating methods or functions.

[RickyC0626]

Ideally, developers should be able to deprecate code they're currently writing a replacement for, even if implementation isn't complete. Plus, developers can update the until parameter, depending on their progress, to help users gauge the approximate version the deprecated code will be fully removed. Users with large projects can benefit from this.

[AntonBallmaier]

Ideally, developers should be able to deprecate code they're currently writing a replacement for, even if implementation isn't complete. Plus, developers can update the until parameter, depending on their progress, to help users gauge the approximate version the deprecated code will be fully removed. Users with large projects can benefit from this.

Ok, both of these things are possible with the current state of the deprecation system. I think I will create a PR soon :)

[huguesdevimeux]

I'm late to the party, but this looks neat. Seeing how complete it will be, it should even be a pip package, since this is not really manim.