Skip to content

[3.10] bpo-46445, bpo-46519: Re-import typing.NewType #30886

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.

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

Merged
merged 1 commit into from
Jan 25, 2022

Conversation

Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Jan 25, 2022

Partially reverts 65b88d5.

https://bugs.python.org/issue46445

Automerge-Triggered-By: GH:Fidget-Spinner

@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting core review labels Jan 25, 2022
@Fidget-Spinner Fidget-Spinner changed the title [3.10] bpo-46445: Re-import typing.NewType [3.10] bpo-46445, bpo-46519: Re-import typing.NewType Jan 25, 2022
@Fidget-Spinner Fidget-Spinner merged commit 9a7d010 into python:3.10 Jan 25, 2022
@Fidget-Spinner Fidget-Spinner deleted the typing_import_newtype branch January 25, 2022 15:31
@gvanrossum
Copy link
Member

The solution seems wrong though. The test goes out of its way to use typing.Foo everywhere.

@Fidget-Spinner
Copy link
Member Author

The solution seems wrong though. The test goes out of its way to use typing.Foo everywhere.

That's on the main branch only right? On 3.10 it's still using plain NewType in many places.

https://github.com/Fidget-Spinner/cpython/blob/9d246cff2fa68301203552806919807b370ccfb1/Lib/test/test_typing.py#L3918
https://github.com/Fidget-Spinner/cpython/blob/9d246cff2fa68301203552806919807b370ccfb1/Lib/test/test_typing.py#L3970

I think we forgot to backport a fix to the tests, so they're now out of sync. I'll try to dig around and backport them, then we can remove this bandaid.

@Fidget-Spinner
Copy link
Member Author

Fidget-Spinner commented Jan 25, 2022

Git blame history goes to:
e89ef0a, then finally
96c4cbd

The last commit is the C accelerator for NewType in 3.11. Which would explain why on main it's importing things as self.module.NewType -- it needs to test both the Python and C version. 3.10 doesn't need that so it should be fine as-is. It was what we used previously after all 65b88d5.

@gvanrossum
Copy link
Member

gvanrossum commented Jan 25, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants