Skip to content

bpo-46413: cover __or__ in _GenericAlias and _SpecialGenericAlias #30640

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 6 commits into from
Jan 19, 2022
Merged

bpo-46413: cover __or__ in _GenericAlias and _SpecialGenericAlias #30640

merged 6 commits into from
Jan 19, 2022

Conversation

@sobolevn
Copy link
Member

@sobolevn sobolevn commented Jan 17, 2022 •
edited by bedevere-bot
Loading

@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting review labels Jan 17, 2022
Fidget-Spinner
Fidget-Spinner reviewed Jan 17, 2022
View reviewed changes
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

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

Thanks for your efforts to increase our code coverage. You might want to check if there are overlaps with https://github.com/python/cpython/blob/main/Lib/test/test_types.py#L628. The tests are in test_types instead of test_typing because the initial implementation used a types.UnionType instead of a typing.Union.

@sobolevn
Copy link
Member Author

sobolevn commented Jan 18, 2022 •
edited
Loading

Yes, seems like some cases are covered there, but not all, like both Callable types.

Thanks for letting me know! 👍

Fidget-Spinner
Fidget-Spinner reviewed Jan 18, 2022
View reviewed changes
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

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

If I understand correctly, the _SpecialGenericAlias and possibly _GenericAlias code paths were not tested previously right? So that's what these tests are trying to cover.

Thanks again, I have a few comments and it'll be ready to merge.

sobolevn and others added 3 commits January 18, 2022 18:43
@sobolevn @Fidget-Spinner
Update Lib/test/test_typing.py
b3b0278 
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
@sobolevn @Fidget-Spinner
Update Lib/test/test_typing.py
e1b32ef 
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
@sobolevn
Update test_typing.py
a6e1926
Fidget-Spinner
Fidget-Spinner reviewed Jan 19, 2022
View reviewed changes
sobolevn and others added 2 commits January 19, 2022 17:39
@sobolevn @Fidget-Spinner
Update Lib/test/test_typing.py
046cf0c 
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
@sobolevn
Update test_typing.py
0ca61d0
@Fidget-Spinner Fidget-Spinner merged commit 0a49148 into python:main Jan 19, 2022
@miss-islington
Copy link
Contributor

miss-islington commented Jan 19, 2022

Thanks @sobolevn for the PR, and @Fidget-Spinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Jan 19, 2022

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

@bedevere-bot
Copy link

bedevere-bot commented Jan 19, 2022

GH-30693 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 19, 2022
@sobolevn @miss-islington
bpo-46413: properly test __{r}or__ code paths in `_SpecialGenericAl…
6b390b7 
…ias` (pythonGH-30640)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0a49148)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 19, 2022
@sobolevn @miss-islington
bpo-46413: properly test __{r}or__ code paths in `_SpecialGenericAl…
269a822 
…ias` (pythonGH-30640)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0a49148)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@bedevere-bot
Copy link

bedevere-bot commented Jan 19, 2022

GH-30694 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 Jan 19, 2022
miss-islington added a commit that referenced this pull request Jan 19, 2022
@miss-islington
[3.10] bpo-46413: properly test __{r}or__ code paths in `_SpecialGe…
39374c4 
…nericAlias` (GH-30640) (GH-30694)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0a49148)


Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

Automerge-Triggered-By: GH:Fidget-Spinner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

+1 more reviewer

@raghavthind2005 raghavthind2005 raghavthind2005 approved these changes

Reviewers whose approvals may not affect merge requirements

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.

6 participants

@sobolevn @miss-islington @bedevere-bot @Fidget-Spinner @raghavthind2005 @the-knights-who-say-ni