Skip to content

bpo-45897: Fix frozen-slotted dataclass bug - #29895

Closed
AlexWaygood wants to merge 16 commits into
python:mainfrom
AlexWaygood:dataclass-slots
Closed

bpo-45897: Fix frozen-slotted dataclass bug#29895
AlexWaygood wants to merge 16 commits into
python:mainfrom
AlexWaygood:dataclass-slots

Conversation

@AlexWaygood

@AlexWaygood AlexWaygood commented Dec 2, 2021 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@AlexWaygood AlexWaygood added the type-bug An unexpected behavior, bug, or error label Dec 6, 2021
arhadthedev
arhadthedev reviewed Dec 15, 2021
View reviewed changes
Comment thread Lib/test/test_dataclasses.py Outdated
erlend-aasland
erlend-aasland reviewed Dec 16, 2021
View reviewed changes
Comment thread Lib/test/test_dataclasses.py Outdated
Comment thread Lib/test/test_dataclasses.py Outdated
Comment thread Lib/test/test_dataclasses.py Outdated
Comment thread Lib/test/test_dataclasses.py Outdated
AlexWaygood and others added 3 commits December 16, 2021 22:37
@AlexWaygood Erlend Egeberg Aasland
Update Lib/test/test_dataclasses.py
9af7074 
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
@AlexWaygood Erlend Egeberg Aasland
Update Lib/test/test_dataclasses.py
9f8c91c 
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
@AlexWaygood
Add comment
ce675c5
erlend-aasland
erlend-aasland reviewed Dec 16, 2021
View reviewed changes
Comment thread Lib/test/test_dataclasses.py Outdated
JelleZijlstra
JelleZijlstra reviewed Jan 24, 2022
View reviewed changes
Comment thread Lib/dataclasses.py
@AlexWaygood
AlexWaygood marked this pull request as draft January 24, 2022 07:48
@AlexWaygood
AlexWaygood marked this pull request as ready for review January 24, 2022 19:20
JelleZijlstra
JelleZijlstra approved these changes Feb 17, 2022
View reviewed changes

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but will leave to @ericvsmith to merge.

Comment thread Lib/dataclasses.py
# Frozen __setattr__/__delattr__ must be added after __slots__
# bpo-45897
if frozen:
for fn in _frozen_get_del_attr(cls, field_list, globals):

@JelleZijlstra JelleZijlstra Feb 17, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this function ought to be named _frozen_set_del_attr. That's outside the scope of this PR though.

@JelleZijlstra

JelleZijlstra commented Apr 13, 2022

Copy link
Copy Markdown
Member

@ericvsmith could you take a look at this PR?

@ericvsmith

ericvsmith commented Apr 13, 2022

Copy link
Copy Markdown
Member

Thanks for the reminder. This is one of the things I'll be reviewing at PyCon.

@AlexWaygood AlexWaygood mentioned this pull request May 3, 2022
Closed
@AlexWaygood

AlexWaygood commented May 3, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Closing and reopening to retrigger CI, since test_concurrent_futures has been running for 2hr43 and counting on Windows (x86) -- I've opened #92222 since this appears to have happened 3 times in 3 days in our CI.

@AlexWaygood AlexWaygood closed this May 3, 2022
@AlexWaygood AlexWaygood reopened this May 3, 2022
@treyhunner treyhunner mentioned this pull request May 3, 2022
Closed
@AlexWaygood AlexWaygood added the needs backport to 3.11 only security fixes label May 8, 2022
@AlexWaygood

AlexWaygood commented Jun 9, 2022

Copy link
Copy Markdown
Member Author

@ericvsmith, any thoughts on this PR? Happy to revise or close if you don't like the approach :)

@ericvsmith

ericvsmith commented Jun 19, 2022

Copy link
Copy Markdown
Member

Sorry it’s taken me so long to look at this, @AlexWaygood. I’ve been super-busy, with no signs of improving. If you don’t hear from me in a two weeks or so, please ping me again.

@AlexWaygood

AlexWaygood commented Jun 19, 2022

Copy link
Copy Markdown
Member Author

Sorry it’s taken me so long to look at this, @AlexWaygood. I’ve been super-busy, with no signs of improving. If you don’t hear from me in a two weeks or so, please ping me again.

Thanks for letting me know @ericvsmith, no worries.

@AlexWaygood

AlexWaygood commented Jul 7, 2022

Copy link
Copy Markdown
Member Author

Sorry it’s taken me so long to look at this, @AlexWaygood. I’ve been super-busy, with no signs of improving. If you don’t hear from me in a two weeks or so, please ping me again.

Ping, @ericvsmith :)

@AlexWaygood

AlexWaygood commented Oct 4, 2022

Copy link
Copy Markdown
Member Author

Closing as per @ericvsmith's rationale in #90055 (comment)

@AlexWaygood AlexWaygood closed this Oct 4, 2022
@AlexWaygood
AlexWaygood deleted the dataclass-slots branch October 4, 2022 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arhadthedev arhadthedev arhadthedev left review comments
@erlend-aasland erlend-aasland erlend-aasland left review comments
@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
@ericvsmith ericvsmith Awaiting requested review from ericvsmith ericvsmith is a code owner

Assignees

No one assigned

Labels

awaiting merge needs backport to 3.11 only security fixes type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@AlexWaygood @JelleZijlstra @ericvsmith @arhadthedev @erlend-aasland @the-knights-who-say-ni @ezio-melotti @bedevere-bot