Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

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

Merged
ambv merged 3 commits into
python:mainpython/cpython:mainfrom
uriyyo:fix-issue-44353-testsuriyyo/cpython:fix-issue-44353-testsCopy head branch name to clipboard
Jul 24, 2021
Merged

bpo-44353: Add tests to cover typing.NewType pickling#27302
ambv merged 3 commits into
python:mainpython/cpython:mainfrom
uriyyo:fix-issue-44353-testsuriyyo/cpython:fix-issue-44353-testsCopy head branch name to clipboard

Conversation

@uriyyo

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

Copy link
Copy Markdown
Member

@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

uriyyo commented Jul 23, 2021

Copy link
Copy Markdown
Member Author

@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
Comment thread Lib/test/test_typing.py Outdated
serhiy-storchaka
serhiy-storchaka reviewed Jul 23, 2021
View reviewed changes
Comment thread Lib/test/test_typing.py Outdated
Comment thread Lib/test/test_typing.py Outdated
@uriyyo

uriyyo commented Jul 23, 2021

Copy link
Copy Markdown
Member Author

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

@serhiy-storchaka

serhiy-storchaka commented Jul 23, 2021

Copy link
Copy Markdown
Member

See also #27311.

@uriyyo

uriyyo commented Jul 23, 2021

Copy link
Copy Markdown
Member Author

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

Should I close this issue?

@serhiy-storchaka

serhiy-storchaka commented Jul 23, 2021

Copy link
Copy Markdown
Member

__reduce__ should use __qualname__, not __name__.

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

@serhiy-storchaka

serhiy-storchaka commented Jul 23, 2021

Copy link
Copy Markdown
Member

You need to copy also changes in constructor and tests.

@uriyyo

uriyyo commented Jul 23, 2021

Copy link
Copy Markdown
Member Author

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

ambv commented Jul 24, 2021

Copy link
Copy Markdown
Contributor

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

miss-islington commented Jul 24, 2021

Copy link
Copy Markdown
Contributor

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

@bedevere-bot

bedevere-bot commented Jul 24, 2021

Copy link
Copy Markdown

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 <1998uriyyo@gmail.com>
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 <1998uriyyo@gmail.com>
@ambv

ambv commented Jul 24, 2021

Copy link
Copy Markdown
Contributor

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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.