KEMBAR78
gh-133925: Make typing._UnionGenericAlias hashable by JelleZijlstra · Pull Request #133929 · python/cpython · GitHub
Skip to content

gh-133925: Make typing._UnionGenericAlias hashable #133929

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 2 commits into from
May 12, 2025
Merged

gh-133925: Make typing._UnionGenericAlias hashable #133929

merged 2 commits into from
May 12, 2025

Conversation

@JelleZijlstra
Copy link
Member

@JelleZijlstra JelleZijlstra commented May 12, 2025 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request May 12, 2025
Closed
AlexWaygood
AlexWaygood reviewed May 12, 2025
View reviewed changes
Lib/typing.py Outdated
Comment on lines 1652 to 1653
def __hash__(self):
return super().__hash__()
Copy link
Member

Choose a reason for hiding this comment

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

This means that a Union instance and a _UnionGenericAlias instance might compare equal but hash differently, right? Might that do counterintuitive things if you used them as dictionary keys or put them in sets?

Copy link
Member

Choose a reason for hiding this comment

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

I think that this is purely about the _UnionGenericAlias class itself, not _UnionGenericAlias instances.

Btw, do we need to add __hash__ tests to instances in other PR?

Copy link
Member

Choose a reason for hiding this comment

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

I think that this is purely about the _UnionGenericAlias class itself, not _UnionGenericAlias instances.

ah, thanks, great point. It still seems like users are storing the classes themselves in sets, though, so I think my point still applies -- the bug report here comes from astropy/astropy#18126

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I should have made it hash() equal to Union. Fixed now.

@JelleZijlstra JelleZijlstra added the needs backport to 3.14 bugs and security fixes label May 12, 2025
@JelleZijlstra JelleZijlstra merged commit 8d478c7 into python:main May 12, 2025
45 checks passed
@miss-islington-app
Copy link

Thanks @JelleZijlstra for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 12, 2025
@JelleZijlstra @miss-islington
pythongh-133925: Make typing._UnionGenericAlias hashable (pythonGH-13…
3efd377 
…3929)

(cherry picked from commit 8d478c7)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented May 12, 2025

GH-133936 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 12, 2025
JelleZijlstra added a commit that referenced this pull request May 12, 2025
@miss-islington @JelleZijlstra
[3.14] gh-133925: Make typing._UnionGenericAlias hashable (GH-133929) (…
89c801b 
…#133936)

gh-133925: Make typing._UnionGenericAlias hashable (GH-133929)
(cherry picked from commit 8d478c7)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@JelleZijlstra @Pranjal095
pythongh-133925: Make typing._UnionGenericAlias hashable (python#133929)
8e42f57
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@JelleZijlstra @taegyunkim
pythongh-133925: Make typing._UnionGenericAlias hashable (python#133929)
eba24a1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sobolevn sobolevn sobolevn approved these changes

@AlexWaygood AlexWaygood AlexWaygood approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@JelleZijlstra @sobolevn @AlexWaygood