-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
gh-90562: Support zero argument super with dataclasses when slots=True #124455
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
Merged
ericvsmith
merged 8 commits into
python:main
from
ericvsmith:issue-90562-dataclasses-zero-argument-super
Sep 25, 2024
Merged
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
aae4c72
Fix zero-argument super() for dataclasses with slots=True.
ericvsmith 97d4f31
Use assertIs instead of assertEqual for class comparisons. Add a tes…
ericvsmith c066fe2
Fix default role.
ericvsmith fbffc50
Improved NEWS entry. Tweak some comments.
ericvsmith 5e1cc0e
Merge branch 'main' into issue-90562-dataclasses-zero-argument-super
ericvsmith 356beed
Fix trivial typo in NEWS.
ericvsmith c801c6d
Apply suggestions from code review
ericvsmith 1c60e6b
Address review comments: Only update one cell, then break out of the …
ericvsmith File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ | |
from typing import ClassVar, Any, List, Union, Tuple, Dict, Generic, TypeVar, Optional, Protocol, DefaultDict | ||
from typing import get_type_hints | ||
from collections import deque, OrderedDict, namedtuple, defaultdict | ||
from functools import total_ordering | ||
from functools import total_ordering, wraps | ||
|
||
import typing # Needed for the string "typing.ClassVar[int]" to work as an annotation. | ||
import dataclasses # Needed for the string "dataclasses.InitVar[int]" to work as an annotation. | ||
|
@@ -4869,5 +4869,125 @@ class A: | |
self.assertEqual(fs[0].name, 'x') | ||
|
||
|
||
class TestZeroArgumentSuperWithSlots(unittest.TestCase): | ||
def test_zero_argument_super(self): | ||
@dataclass(slots=True) | ||
class A: | ||
def foo(self): | ||
super() | ||
|
||
A().foo() | ||
|
||
def test_zero_argument_super_with_old_property(self): | ||
ericvsmith marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@dataclass(slots=True) | ||
class A: | ||
def _get_foo(slf): | ||
return slf.__class__ | ||
|
||
def _set_foo(slf, value): | ||
self.assertIs(__class__, type(slf)) | ||
|
||
def _del_foo(slf): | ||
self.assertIs(__class__, type(slf)) | ||
ericvsmith marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
foo = property(_get_foo, _set_foo, _del_foo) | ||
|
||
a = A() | ||
self.assertIs(a.foo, A) | ||
a.foo = 4 | ||
del a.foo | ||
|
||
def test_zero_argument_super_with_new_property(self): | ||
ericvsmith marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@dataclass(slots=True) | ||
class A: | ||
@property | ||
def foo(slf): | ||
return slf.__class__ | ||
|
||
@foo.setter | ||
def foo(slf, value): | ||
self.assertIs(__class__, type(slf)) | ||
|
||
@foo.deleter | ||
def foo(slf): | ||
self.assertIs(__class__, type(slf)) | ||
|
||
a = A() | ||
self.assertIs(a.foo, A) | ||
a.foo = 4 | ||
del a.foo | ||
|
||
# Test the parts of a property individually. | ||
def test_slots_dunder_class_property_getter(self): | ||
@dataclass(slots=True) | ||
class A: | ||
@property | ||
def foo(slf): | ||
return __class__ | ||
|
||
a = A() | ||
self.assertIs(a.foo, A) | ||
|
||
def test_slots_dunder_class_property_setter(self): | ||
@dataclass(slots=True) | ||
class A: | ||
foo = property() | ||
@foo.setter | ||
def foo(slf, val): | ||
self.assertIs(__class__, type(slf)) | ||
|
||
a = A() | ||
a.foo = 4 | ||
|
||
def test_slots_dunder_class_property_deleter(self): | ||
@dataclass(slots=True) | ||
class A: | ||
foo = property() | ||
@foo.deleter | ||
def foo(slf): | ||
self.assertIs(__class__, type(slf)) | ||
carljm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
a = A() | ||
del a.foo | ||
|
||
def test_wrapped(self): | ||
def mydecorator(f): | ||
@wraps(f) | ||
def wrapper(*args, **kwargs): | ||
return f(*args, **kwargs) | ||
return wrapper | ||
|
||
@dataclass(slots=True) | ||
class A: | ||
@mydecorator | ||
def foo(self): | ||
super() | ||
|
||
A().foo() | ||
|
||
def test_remembered_class(self): | ||
# Apply the dataclass decorator manually (not when the class | ||
# is created), so that we can keep a reference to the | ||
# undecorated class. | ||
class A: | ||
def cls(self): | ||
return __class__ | ||
|
||
self.assertIs(A().cls(), A) | ||
|
||
B = dataclass(slots=True)(A) | ||
self.assertIs(B().cls(), B) | ||
|
||
# This is probably undesirable behavior, but is a function of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can safely remove this "probably" :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heh. Will do. |
||
# how modifying __class__ in the closure works. I'm not sure | ||
# this should be tested or not: I don't really want to | ||
# guarantee this behavior, but I don't want to lose the point | ||
# that this is how it works. | ||
|
||
# The underlying class is "broken" by changing its __class__ | ||
# in A.foo() to B. This normally isn't a problem, because no | ||
# one will be keeping a reference to the underlying class A. | ||
carljm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.assertIs(A().cls(), B) | ||
|
||
if __name__ == '__main__': | ||
unittest.main() |
3 changes: 3 additions & 0 deletions
3
Misc/NEWS.d/next/Library/2024-09-23-18-26-17.gh-issue-90562.Yj566G.rst
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
Modify dataclasses to support zero-argument super() when ``slots=True`` is | ||
specified. This works by modifying all references to ``__class__`` to point | ||
to the newly created class. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think technically we could break out of the loop as soon as we've successfully updated one cell, because all the closures for a given class will always share a single
__class__
cell. This may improve performance noticeably for larger classes?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Will change.