laurent-laporte-pro / deprecated

Python @deprecated decorator to deprecate old python classes, functions or methods.
MIT License
305 stars 34 forks source link

[feature request] Deprecate names imported with `from b import *` #54

Closed kc9jud closed 1 year ago

kc9jud commented 2 years ago

Expected Behavior

Some old code imported everything from a submodule into the top-level namespace:

# a/__init__.py:
from . import b
from .b import *

We want calling code to use a.b.foo() instead of a.foo(), but do it gently.

Actual Behavior

Basically, we want direct use of a.foo() to be deprecated, but without having list manually list all of the imported names. I currently have this implemented with importlib:

################################################################
# import from b into namespace but mark as deprecated
################################################################
import importlib as __importlib
import deprecated as __deprecated
import types as __types
__b_module = __importlib.import_module('.b', __name__)

# is there an __all__?  if so respect it
if "__all__" in __b_module.__dict__:
    __names = __b_module.__dict__["__all__"]
else:
    # otherwise we import all names that don't begin with _
    __names = [x for x in __b_module.__dict__ if not x.startswith("_")]

# decorate the items as they get pulled into the namespace
globals().update({
    k: (
        __deprecated.deprecated(getattr(__b_module, k), action="always", reason="use `b` namespace")
        if not isinstance(getattr(__b_module, k), __types.ModuleType)
        else getattr(__b_module, k)
    )
    for k in __names
    })

It would be nice to have a way to wrap this into a function or decorator or something similar, so that I could simply do something like:

deprecated.import_all('.b', action="always", reason="use `b` namespace")

I'm not sure how this works with the magic of globals() though.

Environment

tantale commented 2 years ago

First, in your a/__init__.py file, if b is a submodule, there is no need to import it directly. In other words, you can remove the line:

from . import b

If you use Python 3.7 or above, you can consider implementing a __getattr__ function into your a/__init__.py file in order to emit a warning message when the user tries to call on of the b methods directly. You should read the PEP-562 to learn more about that.

The __getattr__ function at the module level should accept one argument which is the name of an attribute and return the computed value or raise an AttributeError.

We can implement this as follows:

# a/__init__.py
import warnings

from . import b as _deprecated_b

def __getattr__(name: str):
    deprecated_attrs = {attr for attr in dir(_deprecated_b) if not attr.startswith("__")}
    if name in deprecated_attrs:
        msg = (
            f"calling '{__name__}.{name}' directly is deprecated,"
            f" you should call '{_deprecated_b.__name__}.{name}' instead"
        )
        warnings.warn(msg, category=DeprecationWarning, stacklevel=2)
        return getattr(_deprecated_b, name)
    try:
        return globals()[name]
    except KeyError:
        raise AttributeError(f"{__name__}.{name}") from None

Here is a simple demonstration:

import warnings

import a

def main():
    a.foo()  # should emit a deprecation warning
    a.b.foo()
    a.b.bar()

if __name__ == '__main__':
    # always print warnings for this demo
    warnings.simplefilter("always")
    main()

When running this script, you should see:

foo
foo
bar
demo.py:7: DeprecationWarning: calling 'a.foo' directly is deprecated, you should call 'a.b.foo' instead
  a.foo()  # should emit a deprecation warning

here: foo() prints "foo" and bar() prints "bar".

kc9jud commented 2 years ago

First, in your a/__init__.py file, if b is a submodule, there is no need to import it directly. In other words, you can remove the line:

from . import b

That requires the calling code to do an explicit import a.b in addition to import a, so that's really a separate issue:

>>> import a
>>> a.b.foo()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'a' has no attribute 'b'
>>> import a.b
>>> a.b.foo()
foo

If you use Python 3.7 or above, you can consider implementing a __getattr__ function into your a/__init__.py file in order to emit a warning message when the user tries to call on of the b methods directly. You should read the PEP-562 to learn more about that.

The __getattr__ function at the module level should accept one argument which is the name of an attribute and return the computed value or raise an AttributeError.

* If the attribute belongs to module `a`, we can return it.

* If the attribute belongs to module `b`, we can emit a `DeprecationWarning` to warn the user,
  and return that attribute.

* If the attribute belongs neither to module `a` nor to module `b`, we must raise `AttributeError`.

We can implement this as follows:

# a/__init__.py
import warnings

from . import b as _deprecated_b

def __getattr__(name: str):
    deprecated_attrs = {attr for attr in dir(_deprecated_b) if not attr.startswith("__")}
    if name in deprecated_attrs:
        msg = (
            f"calling '{__name__}.{name}' directly is deprecated,"
            f" you should call '{_deprecated_b.__name__}.{name}' instead"
        )
        warnings.warn(msg, category=DeprecationWarning, stacklevel=2)
        return getattr(_deprecated_b, name)
    try:
        return globals()[name]
    except KeyError:
        raise AttributeError(f"{__name__}.{name}") from None

This looks like it doesn't respect b.__all__, but that can be fixed by replacing dir(_deprecated_b) with getattr(_deprecated_b, "__all__", dir(_deprecated_b)), right?

Here is a simple demonstration:

import warnings

import a

def main():
    a.foo()  # should emit a deprecation warning
    a.b.foo()
    a.b.bar()

if __name__ == '__main__':
    # always print warnings for this demo
    warnings.simplefilter("always")
    main()

When running this script, you should see:

foo
foo
bar
demo.py:7: DeprecationWarning: calling 'a.foo' directly is deprecated, you should call 'a.b.foo' instead
  a.foo()  # should emit a deprecation warning

here: foo() prints "foo" and bar() prints "bar".

Is there a way to wrap this nicely into, e.g., a factory function within deprecated? This is awfully verbose...

tantale commented 2 years ago

I'm not going to continue talking about the from . import b problem; because this is an implementation choice (which I don't share) that is not related to module attribute deprecation.

Otherwise, I have started working on designing a function to declare module attributes as deprecated. In the case of module attributes, you can't define a decorator (because it's not possible to decorate an attribute). So, the best thing to do is to define a small function, say deprecate_attr to emit a deprecation warning. An example of use could be:

# in your module:
from deprecated.attrs import deprecate_attr

flag = 0

deprecate_attr("flag", reason="No more used", version="0.2.0")

Implementing a function is not trivial because, in order for it to work using __getattr__, the deprecated attributes must be removed from the module's globals() (so that __getattr__ can actually be called. You also need to implement __dir__...

tantale commented 1 year ago

Module-level depreciation is currently being developed. Stay tune.