Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deprecating a mixin #294

Merged
merged 8 commits into from Nov 4, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -3,6 +3,8 @@
- All parameters on `NewType.__call__` are now positional-only. This means that
the signature of `typing_extensions.NewType.__call__` now exactly matches the
signature of `typing.NewType.__call__`. Patch by Alex Waygood.
- Fix bug with using `@deprecated` on a mixin class. Inheriting from a
deprecated class now raises a `DeprecationWarning`. Patch by Jelle Zijlstra.

# Release 4.8.0 (September 17, 2023)

Expand Down
5 changes: 5 additions & 0 deletions doc/index.rst
Expand Up @@ -553,6 +553,11 @@ Decorators

.. versionadded:: 4.5.0

.. versionchanged:: 4.9.0

Inheriting from a deprecated class now also raises a runtime
:py:exc:`DeprecationWarning`.

.. decorator:: final

See :py:func:`typing.final` and :pep:`591`. In ``typing`` since 3.8.
Expand Down
52 changes: 52 additions & 0 deletions src/test_typing_extensions.py
Expand Up @@ -418,6 +418,58 @@ def __new__(cls, x):
self.assertEqual(instance.x, 42)
self.assertTrue(new_called)

def test_mixin_class(self):
@deprecated("Mixin will go away soon")
class Mixin:
pass

class Base:
def __init__(self, a) -> None:
self.a = a

with self.assertWarnsRegex(DeprecationWarning, "Mixin will go away soon"):
class Child(Base, Mixin):
pass

instance = Child(42)
self.assertEqual(instance.a, 42)

def test_existing_init_subclass(self):
@deprecated("C will go away soon")
class C:
def __init_subclass__(cls) -> None:
cls.inited = True

with self.assertWarnsRegex(DeprecationWarning, "C will go away soon"):
C()

with self.assertWarnsRegex(DeprecationWarning, "C will go away soon"):
class D(C):
pass

self.assertTrue(D.inited)
self.assertIsInstance(D(), D) # no deprecation

def test_existing_init_subclass_in_base(self):
class Base:
def __init_subclass__(cls, x) -> None:
cls.inited = x

@deprecated("C will go away soon")
class C(Base, x=42):
pass

self.assertEqual(C.inited, 42)

with self.assertWarnsRegex(DeprecationWarning, "C will go away soon"):
C()

with self.assertWarnsRegex(DeprecationWarning, "C will go away soon"):
class D(C, x=3):
pass

self.assertEqual(D.inited, 3)

def test_function(self):
@deprecated("b will go away soon")
def b():
Expand Down
15 changes: 12 additions & 3 deletions src/typing_extensions.py
Expand Up @@ -2337,21 +2337,30 @@ def decorator(arg: _T, /) -> _T:
return arg
elif isinstance(arg, type):
original_new = arg.__new__
has_init = arg.__init__ is not object.__init__
original_init_subclass = arg.__init_subclass__

@functools.wraps(original_new)
def __new__(cls, *args, **kwargs):
warnings.warn(msg, category=category, stacklevel=stacklevel + 1)
if cls is arg:
warnings.warn(msg, category=category, stacklevel=stacklevel + 1)
if original_new is not object.__new__:
return original_new(cls, *args, **kwargs)
# Mirrors a similar check in object.__new__.
elif not has_init and (args or kwargs):
elif cls.__init__ is object.__init__ and (args or kwargs):
raise TypeError(f"{cls.__name__}() takes no arguments")
else:
return original_new(cls)

arg.__new__ = staticmethod(__new__)

@functools.wraps(original_init_subclass)
def __init_subclass__(*args, **kwargs):
warnings.warn(msg, category=category, stacklevel=stacklevel + 1)
return original_init_subclass(*args, **kwargs)

arg.__init_subclass__ = __init_subclass__
arg.__deprecated__ = __new__.__deprecated__ = msg
__init_subclass__.__deprecated__ = msg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't always do the right thing, because of the fact that __init_subclass__ is an implicit classmethod. For example:

Python 3.11.5 (tags/v3.11.5:cce6ba9, Aug 24 2023, 14:38:34) [MSC v.1936 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import typing_extensions as t
>>> @t.deprecated('DEPRECATED')
... class Foo:
...     ATTR = 1
...     def __init_subclass__(cls, *args, **kwargs):
...         print(f"{cls=}, {cls.ATTR=}")
...
>>> class Bar:
...     ATTR = 2
...
>>> class Baz(Bar, Foo): ...
...
<stdin>:1: DeprecationWarning: DEPRECATED
cls=<class '__main__.Foo'>, cls.ATTR=1
>>> Baz.ATTR
2

You can fix it by doing this (diff is relative to your PR branch):

diff --git a/src/typing_extensions.py b/src/typing_extensions.py
index 6a0015a..37ef575 100644
--- a/src/typing_extensions.py
+++ b/src/typing_extensions.py
@@ -2337,7 +2337,7 @@ else:
                 return arg
             elif isinstance(arg, type):
                 original_new = arg.__new__
-                original_init_subclass = arg.__init_subclass__
+                original_init_subclass = arg.__init_subclass__.__func__

                 @functools.wraps(original_new)
                 def __new__(cls, *args, **kwargs):
@@ -2358,7 +2358,7 @@ else:
                     warnings.warn(msg, category=category, stacklevel=stacklevel + 1)
                     return original_init_subclass(*args, **kwargs)

-                arg.__init_subclass__ = __init_subclass__
+                arg.__init_subclass__ = classmethod(__init_subclass__)
                 arg.__deprecated__ = __new__.__deprecated__ = msg
                 __init_subclass__.__deprecated__ = msg
                 return arg

With this change, we get the correct behaviour:

>>> import typing_extensions as t
>>> @t.deprecated('DEPRECATED')
... class Foo:
...     ATTR = 1
...     def __init_subclass__(cls, *args, **kwargs):
...         print(f"{cls=}, {cls.ATTR=}")
...
>>> class Bar:
...     ATTR = 2
...
>>> class Baz(Bar, Foo): ...
...
<stdin>:1: DeprecationWarning: DEPRECATED
cls=<class '__main__.Baz'>, cls.ATTR=2

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this. Your solution isn't quite right as it doesn't work for classes without a Python __init_subclass__, but I got something to work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, thanks -- I checked my solution worked in the REPL, but didn't actually run the test suite with my changes :) My bad!

return arg
elif callable(arg):
@functools.wraps(arg)
Expand Down