| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Would we be able to get rid of typing._make_union, and just have TypeVar etc. call UnionType.__class_getitem__ directly? |
Sorry, something went wrong.
Yes |
Sorry, something went wrong.
|
Instead of deleting typing.Union completely, adding UnionType.__class_getitem__, and making typing.Union an alias for types.UnionType, I wonder if we could just change typing.Union so that it's a special form that just returns instances of UnionType: from operator import or_
from functools import reduce
@_SpecialForm
def Union(self, parameters):
return reduce(or_, parameters)That would avoid the issue of "Should we rename types.UnionType to be Union", as we'd keep them as distinct objects. But I know your plan is to deal with handling forward refs in | expressions later, which would mean that^ idea above wouldn't work right now (unions involving forward references would break). So perhaps you could instead expose a _secret_undocumented_constructor method for types.UnionType (we can bikeshed over the name), and then just do @_SpecialForm
def Union(self, parameters):
return types.UnionType._secret_undocumented_constructor(parameters)Or we could just expose the constructor of types.UnionType, of course. |
Sorry, something went wrong.
|
Also you can now get rid of this function as part of this PR; prior callers of the function can now just use isinstance() checks against types.UnionType: Lines 842 to 844 in 6a8b862 |
Sorry, something went wrong.
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 get_origin(Union[int, str]) != Union. |
Sorry, something went wrong.
|
I guess one of my reservations here is that UnionType ends up looking like a pretty weird type. You can't construct instances of it directly: >>> 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 __class_getitem__ backdoor" to create instances directly: >>> UnionType[int, str, bytes]
int | str | bytesIt's very unusual for __class_getitem__ to just directly return instances of the class, and it feels really weird to make that the only way you're allowed to construct instances of the class. Maybe that means we should just expose the constructor as well as adding __class_getitem__...?
Good point, that would indeed be confusing. |
Sorry, something went wrong.
|
I can make the constructor work. Should it take *args and union them all together? |
Sorry, something went wrong.
That makes sense to me! |
Sorry, something went wrong.
There was a problem hiding this comment.
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:
from types import UnionType
from typing import Union
UnionType[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.
Sorry, something went wrong.
| 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.
.__args__ is no longer writable.
Sorry, something went wrong.
|
|
||
| bt = BadType('bt', (), {}) | ||
| bt2 = BadType('bt2', (), {}) | ||
| # Comparison should fail and errors should propagate out for bad types. |
There was a problem hiding this comment.
With the new code there are fewer code paths that trigger the equality comparison.
Sorry, something went wrong.
|
It's been long enough, I'm planning to merge this once the tests pass again. |
Sorry, something went wrong.
|
🤖 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. |
Sorry, something went wrong.
|
This PR causes a potential regression: #131933. |
Sorry, something went wrong.
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.
| Back | FazBrowse Home | New Git URL |
📚 Documentation preview 📚: https://cpython-previews--105511.org.readthedocs.build/