Skip to content

[3.9] bpo-45520: Backport __getstate__, __setstate__ methods for dataclasses from 3.10 into 3.9 (GH-25786) #29147

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[3.9] bpo-45520: Backport __getstate__, __setstate__ methods for dataclasses from 3.10 into 3.9 (GH-25786) #29147

wants to merge 2 commits into from

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Oct 22, 2021 •
edited
Loading

@sobolevn sobolevn requested a review from ericvsmith as a code owner October 22, 2021 08:04
@sobolevn sobolevn changed the title Backport __getstate__, __setstate__ methods from 3.10 into 3.9 [3.9] bpo-45520: Backport __getstate__, __setstate__ methods from 3.10 into 3.9 (GH-25786) Oct 22, 2021
@sobolevn sobolevn closed this Oct 22, 2021
@sobolevn sobolevn reopened this Oct 22, 2021
@sobolevn
Copy link
Member Author

sobolevn commented Oct 22, 2021 •
edited
Loading

I'll toggle this PR once again after all other tests pass, because Azure seems to be broken for some unrelated reason.

@sobolevn
Copy link
Member Author

It now passes!

I've also created https://bugs.python.org/issue45566 to sync the pickle protocols thing with more recent versions.

@sobolevn sobolevn mentioned this pull request Oct 22, 2021
Merged
miss-islington pushed a commit that referenced this pull request Oct 24, 2021
@sobolevn
bpo-45566: test_frozen_pickle checks all pickle protocols (GH-29150)
07236d5 
Refs #29147

Automerge-Triggered-By: GH:ericvsmith
@miss-islington miss-islington mentioned this pull request Oct 24, 2021
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 24, 2021
@sobolevn @miss-islington
bpo-45566: test_frozen_pickle checks all pickle protocols (python…
a85231d 
…GH-29150)

Refs python#29147

Automerge-Triggered-By: GH:ericvsmith
(cherry picked from commit 07236d5)

Co-authored-by: Nikita Sobolev <[email protected]>
miss-islington added a commit that referenced this pull request Oct 24, 2021
@miss-islington @sobolevn
bpo-45566: test_frozen_pickle checks all pickle protocols (GH-29150)
36971fd 
Refs #29147

Automerge-Triggered-By: GH:ericvsmith
(cherry picked from commit 07236d5)

Co-authored-by: Nikita Sobolev <[email protected]>
@sobolevn
Copy link
Member Author

Friendly ping @ericvsmith 🙂

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Nov 29, 2021
@sobolevn
Copy link
Member Author

CC @ericvsmith @Fidget-Spinner

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Nov 30, 2021
@merwok
Copy link
Member

merwok commented Jan 2, 2022

Please mention dataclasses in the PR title

@sobolevn sobolevn changed the title [3.9] bpo-45520: Backport __getstate__, __setstate__ methods from 3.10 into 3.9 (GH-25786) [3.9] bpo-45520: Backport __getstate__, __setstate__ methods for dataclasses from 3.10 into 3.9 (GH-25786) Jan 2, 2022
@hugovk hugovk mentioned this pull request Jan 5, 2022
Closed
@Fidget-Spinner
Copy link
Member

@sobolevn sorry for the really long delay (I dropped the ball on this notification for a month). Unfortunately I have to admit I'm clueless at pickle so I can't review this.

@sobolevn
Copy link
Member Author

sobolevn commented Jan 12, 2022 •
edited
Loading

@Fidget-Spinner no worries, thanks for your work!

This is a backport of a bugfix from 3.10 to 3.9
For some reason it was not done in #25786

CC @JelleZijlstra as the original reviewer 🙂

@JelleZijlstra
Copy link
Member

Since dataclasses only gained native slot support in 3.10, it doesn't seem important to apply this fix to 3.9. (It affects people who set __slots__ manually, but if you do that it doesn't seem too bad to also manually provide pickle support.) Also, unconditionally setting these methods is risky; it may overwrite a user-provided method.

I'll leave the decision to @ericvsmith though.

MaxwellDupre
MaxwellDupre approved these changes Apr 19, 2022
View reviewed changes
Copy link
Contributor

@MaxwellDupre MaxwellDupre left a comment

Choose a reason for hiding this comment

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

Ran 175 tests in 1.202s
OK
Looks reasonable.

@ambv
Copy link
Contributor

ambv commented May 16, 2022

We're releasing the last bugfix release of 3.9 today and this PR's been stale for a while. Sadly it's a wontfix now.

@ambv ambv closed this May 16, 2022
@jfuruness jfuruness mannequin mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaxwellDupre MaxwellDupre MaxwellDupre approved these changes

@ericvsmith ericvsmith Awaiting requested review from ericvsmith ericvsmith is a code owner

Assignees
No one assigned
Labels
awaiting core review skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@sobolevn @merwok @Fidget-Spinner @JelleZijlstra @ambv @MaxwellDupre @the-knights-who-say-ni @bedevere-bot