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.

Sign up for GitHub

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

gh-102578: Optimise setting and deleting mutable attributes on non-dataclass subclasses of frozen dataclasses #102573

Merged
merged 15 commits into from
Mar 11, 2023
Merged

gh-102578: Optimise setting and deleting mutable attributes on non-dataclass subclasses of frozen dataclasses #102573

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
776843d
Use `set`-based name lookup rather than `tuple`s for frozen dataclasses
XuehaiPan Mar 10, 2023
0a400c1
Merge branch 'main' into dataclasses-lookup
XuehaiPan Mar 10, 2023
54dd9c3
Add news entry
XuehaiPan Mar 10, 2023
845c8ff
Update new entry
XuehaiPan Mar 10, 2023
0f0d97c
Use empty tuple singleton in simple case
XuehaiPan Mar 10, 2023
0524ee4
Update new entry
XuehaiPan Mar 10, 2023
4055fa3
Add more details about the change in news entry
XuehaiPan Mar 10, 2023
0bbf3a8
Update news entry
XuehaiPan Mar 10, 2023
05eae2c
Merge branch 'main' into dataclasses-lookup
XuehaiPan Mar 10, 2023
b90d251
Eliminate unnecessary check for empty fields
XuehaiPan Mar 10, 2023
d3c8403
Remove mistake from auto import
XuehaiPan Mar 10, 2023
84b5bdd
Add test for empty frozen dataclass
XuehaiPan Mar 10, 2023
2d38e9c
Add test for empty frozen dataclass
XuehaiPan Mar 10, 2023
ef8e83f
Add test for empty frozen dataclass
XuehaiPan Mar 10, 2023
5da787e
Add extra check for the exception type
XuehaiPan Mar 10, 2023
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
5 changes: 3 additions & 2 deletions Lib/dataclasses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,9 +617,10 @@ def _frozen_get_del_attr(cls, fields, globals):
locals = {'cls': cls,
'FrozenInstanceError': FrozenInstanceError}
if fields:
fields_str = '(' + ','.join(repr(f.name) for f in fields) + ',)'
fields_str = '{' + ','.join(repr(f.name) for f in fields) + '}'
else:
# Special case for the zero-length tuple.
# Special case for the zero-length set.
# Use the empty tuple singleton to avoid unnecessary `set` construction
Copy link
Member

Choose a reason for hiding this comment

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

Not that it matters much, but the zero length case could just avoid the or name in ... test entirely. Maybe fields_str should become fields_test, and then set it to or name in {<<generated set literal>>} or set it to an empty string if there are no fields. Then change the generated code to f'if type(self) is cls {fields_test}:' Although that doesn't read very well. Maybe tweak fields_test to be something else.

This could be part of a different PR, or include it here. But in any event I'm not positive that the zero length case actually has a test. We should make sure it does for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But in any event I'm not positive that the zero length case actually has a test. We should make sure it does for this PR.

Added a small test for empty frozen dataclass.

fields_str = '()'
return (_create_fn('__setattr__',
('self', 'name', 'value'),
Expand Down
4 changes: 4 additions & 0 deletions Misc/NEWS.d/next/Library/2023-03-10-13-21-16.gh-issue-102578.-gujoI.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Speed up setting or deleting mutable attributes on non-dataclass subclasses of
frozen dataclasses. Due to the implementation of ``__setattr__`` and
``__delattr__`` for frozen dataclasses, this previously had a time complexity
of ``O(n)``. It now has a time complexity of ``O(1)``.