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 @deprecated on classes with only __new__ #193

Merged
merged 1 commit into from
May 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
# Unreleased

- Fix regression in version 4.6.1 where comparing a generic class against a
- Fix use of `@deprecated` on classes with `__new__` but no `__init__`.
Patch by Jelle Zijlstra.
- Fix regression in version 4.6.1 where comparing a generic class against a
runtime-checkable protocol using `isinstance()` would cause `AttributeError`
to be raised if using Python 3.7
to be raised if using Python 3.7.

# Release 4.6.1 (May 23, 2023)

Expand Down
21 changes: 21 additions & 0 deletions src/test_typing_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ class A:
with self.assertRaises(TypeError):
A(42)

def test_class_with_init(self):
@deprecated("HasInit will go away soon")
class HasInit:
def __init__(self, x):
Expand All @@ -366,6 +367,7 @@ def __init__(self, x):
instance = HasInit(42)
self.assertEqual(instance.x, 42)

def test_class_with_new(self):
has_new_called = False

@deprecated("HasNew will go away soon")
Expand All @@ -382,6 +384,8 @@ def __init__(self, x) -> None:
instance = HasNew(42)
self.assertEqual(instance.x, 42)
self.assertTrue(has_new_called)

def test_class_with_inherited_new(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This name is different from test_inherit_from_class_with_new in the CPython PR, but I guess it's also missing some of the tests in that PR

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll have to update the CPython PR with these changes if the PEP does get accepted.

new_base_called = False

class NewBase:
Expand All @@ -402,6 +406,23 @@ class HasInheritedNew(NewBase):
self.assertEqual(instance.x, 42)
self.assertTrue(new_base_called)

def test_class_with_new_but_no_init(self):
new_called = False

@deprecated("HasNewNoInit will go away soon")
class HasNewNoInit:
def __new__(cls, x):
nonlocal new_called
new_called = True
obj = super().__new__(cls)
obj.x = x
return obj

with self.assertWarnsRegex(DeprecationWarning, "HasNewNoInit will go away soon"):
instance = HasNewNoInit(42)
self.assertEqual(instance.x, 42)
self.assertTrue(new_called)

def test_function(self):
@deprecated("b will go away soon")
def b():
Expand Down
6 changes: 3 additions & 3 deletions src/typing_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -2497,11 +2497,11 @@ def decorator(__arg: _T) -> _T:
@functools.wraps(original_new)
def __new__(cls, *args, **kwargs):
warnings.warn(__msg, category=category, stacklevel=stacklevel + 1)
# Mirrors a similar check in object.__new__.
if not has_init and (args or kwargs):
raise TypeError(f"{cls.__name__}() takes no arguments")
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):
raise TypeError(f"{cls.__name__}() takes no arguments")
else:
return original_new(cls)

Expand Down