Skip to content

bpo-44353: Add tests to cover typing.NewType pickling #27302

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
Merged
merged 3 commits into from
Jul 24, 2021
Merged

bpo-44353: Add tests to cover typing.NewType pickling #27302

merged 3 commits into from
Jul 24, 2021

Conversation

uriyyo
Copy link
Member

@uriyyo uriyyo commented Jul 23, 2021 •
edited by bedevere-bot
Loading

@uriyyo uriyyo requested a review from gvanrossum as a code owner July 23, 2021 10:11
@bedevere-bot bedevere-bot added the tests Tests in the Lib/test dir label Jul 23, 2021
@uriyyo
Copy link
Member Author

uriyyo commented Jul 23, 2021

@ambv Could you please review this PR and add skip-news label?

@ambv ambv added the skip news label Jul 23, 2021
ambv
ambv reviewed Jul 23, 2021
View reviewed changes
serhiy-storchaka
serhiy-storchaka reviewed Jul 23, 2021
View reviewed changes
@uriyyo
Copy link
Member Author

uriyyo commented Jul 23, 2021

@serhiy-storchaka What should I do with this PR?

@serhiy-storchaka
Copy link
Member

See also #27311.

@uriyyo
Copy link
Member Author

uriyyo commented Jul 23, 2021

As far as I see #27311 will include code from this PR.

Should I close this issue?

@serhiy-storchaka
Copy link
Member

__reduce__ should use __qualname__, not __name__.

You can either copy pickle-related code from #27311 or close this issue.

@serhiy-storchaka
Copy link
Member

You need to copy also changes in constructor and tests.

@uriyyo
Copy link
Member Author

uriyyo commented Jul 23, 2021

Done, changes from your PR has been copied to this

@ambv ambv force-pushed the fix-issue-44353-tests branch from 3563d76 to 5c6cd2b Compare July 24, 2021 09:29
@ambv
Copy link
Contributor

ambv commented Jul 24, 2021

Rebased over main after GH-27311 got merged.

@ambv ambv merged commit a22b05d into python:main Jul 24, 2021
@ambv ambv added the needs backport to 3.10 only security fixes label Jul 24, 2021
@miss-islington
Copy link
Contributor

Thanks @uriyyo for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-27328 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 24, 2021
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 24, 2021
@uriyyo @miss-islington
bpo-44353: Improve tests covering typing.NewType pickling (pythonGH-2…
69130dd 
…7302)

(cherry picked from commit a22b05d)

Co-authored-by: Yurii Karabas <[email protected]>
ambv pushed a commit that referenced this pull request Jul 24, 2021
@miss-islington @uriyyo
bpo-44353: Improve tests covering typing.NewType pickling (GH-27302) (G…
e8c0174 
…H-27328)

(cherry picked from commit a22b05d)

Co-authored-by: Yurii Karabas <[email protected]>
@ambv
Copy link
Contributor

ambv commented Jul 24, 2021

Thanks a lot Yurii! ✨ 🍰 ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ambv ambv ambv left review comments

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

@Fidget-Spinner Fidget-Spinner Awaiting requested review from Fidget-Spinner

@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka

Assignees
No one assigned
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@uriyyo @serhiy-storchaka @ambv @miss-islington @bedevere-bot @JelleZijlstra @Fidget-Spinner @the-knights-who-say-ni