Skip to content

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

Closed
sobolevn wants to merge 2 commits intopython:3.9from
sobolevn:issue-45520
Closed

[3.9] bpo-45520: Backport __getstate__, __setstate__ methods for dataclasses from 3.10 into 3.9 (GH-25786)#29147
sobolevn wants to merge 2 commits intopython:3.9from
sobolevn:issue-45520

Conversation

@sobolevn
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

sobolevn commented Oct 22, 2021

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 <mail@sobolevn.me>
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 <mail@sobolevn.me>
@sobolevn
Copy link
Copy Markdown
Member Author

sobolevn commented Oct 29, 2021

Friendly ping @ericvsmith 🙂

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 29, 2021

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
Copy Markdown
Member Author

sobolevn commented Nov 29, 2021

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
Copy Markdown
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
Copy Markdown
Member

Fidget-Spinner commented Jan 12, 2022

@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
Copy Markdown
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
Copy Markdown
Member

JelleZijlstra commented Feb 23, 2022

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
Copy Markdown
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
Copy Markdown
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

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

1 more reviewer

@MaxwellDupre MaxwellDupre MaxwellDupre approved these changes

Reviewers whose approvals may not affect merge requirements

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