Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-105499: Merge typing.Union and types.UnionType#105511
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
JelleZijlstra commented Jun 8, 2023 • edited by github-actions bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by github-actions bot
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
AlexWaygood commented Jun 8, 2023
Would we be able to get rid of |
JelleZijlstra commented Jun 8, 2023
Yes |
AlexWaygood commented Jun 8, 2023
Instead of deleting fromoperatorimportor_fromfunctoolsimportreduce@_SpecialFormdefUnion(self, parameters): returnreduce(or_, parameters)That would avoid the issue of "Should we rename But I know your plan is to deal with handling forward refs in So perhaps you could instead expose a @_SpecialFormdefUnion(self, parameters): returntypes.UnionType._secret_undocumented_constructor(parameters)Or we could just expose the constructor of |
AlexWaygood commented Jun 8, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Also you can now get rid of this function as part of this PR; prior callers of the function can now just use Lines 842 to 844 in 6a8b862
|
JelleZijlstra commented Jun 8, 2023
I'd rather not keep them as distinct objects, though; that means we still have two objects that to a user look like the same thing. Plus, we'd have |
AlexWaygood commented Jun 8, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I guess one of my reservations here is that >>> from types import UnionType >>> UnionType(int, str) Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: cannot create 'types.UnionType' instances...Except wait, you can, we now have a "secret >>> UnionType[int, str, bytes] int | str | bytesIt's very unusual for Maybe that means we should just expose the constructor as well as adding
Good point, that would indeed be confusing. |
JelleZijlstra commented Jun 8, 2023
I can make the constructor work. Should it take *args and union them all together? |
AlexWaygood commented Jun 8, 2023
That makes sense to me! |
AlexWaygood left a comment • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, in my opinion. From a design perspective, the only weirdnesses I can see are that both of these become valid at runtime:
fromtypesimportUnionTypefromtypingimportUnionUnionType[int, str] Union(int, str)I can live with that, though, and hopefully linters can flag those uses. (Type checkers almost certainly will, anyway.) On our side, we can just not document that you can do either of those things.
JelleZijlstra commented Sep 28, 2024
Finally got around to fixing hashability, and I'm pretty happy with the result (less code than before!). At the sprint I finagled @AlexWaygood into maybe taking another look. I still think this is a good idea and we should move forward with it. |
| x<y | ||
| # Check that we don't crash if typing.Union does not have a tuple in __args__ | ||
| y=typing.Union[str, int] | ||
| y.__args__= [str, int] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.__args__ is no longer writable.
Uh oh!
There was an error while loading. Please reload this page.
| bt=BadType('bt', (),{}) | ||
| bt2=BadType('bt2', (),{}) | ||
| # Comparison should fail and errors should propagate out for bad types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the new code there are fewer code paths that trigger the equality comparison.
JelleZijlstra commented Mar 3, 2025
It's been long enough, I'm planning to merge this once the tests pass again. |
bedevere-bot commented Mar 3, 2025
🤖 New build scheduled with the buildbot fleet by @JelleZijlstra for commit cab69f0 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F105511%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
dc6d66f into python:mainUh oh!
There was an error while loading. Please reload this page.
hugovk commented Mar 31, 2025
This PR causes a potential regression: #131933. |
Leftover from python#105511 I believe. GitHub code search found no usages other than copies of typing.py and lists of stdlib functions.
…s.UnionType (python#105511)" This reverts commit d1db43c. This reverts commit 0f511d8. This reverts commit dc6d66f.
…s.UnionType (python#105511)" This reverts commit d1db43c. This reverts commit 0f511d8. This reverts commit dc6d66f.
typing.Unionandtypes.UnionType#105499📚 Documentation preview 📚: https://cpython-previews--105511.org.readthedocs.build/