Skip to content

PEP 604: Add or operator to class type #5151

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 6 commits into from
Apr 8, 2021
Merged

Conversation

mano7onam
Copy link
Contributor

Python 3.10 allowing writing union types as X | Y so class type now should have operator or

@jakebailey
Copy link
Contributor

jakebailey commented Mar 29, 2021

Does this need to be generic in some way to annotate that the return type is a Union, or is that a detail that does not matter for type checkers as they special case it?

e.g. (spitballing, don't take this as correct without someone else to vet me...):

_TT = TypeVar("_TT", bound="type")
_TT2 = TypeVar("_TT2", bound="type")

def __or__(self: _TT, t: _TT2) -> Union[_TT, _TT2]: ...

Note that no type checker runs for 3.10 as far as I'm aware, so there isn't much visibility into breakages caused by adding 3.10-related code. I sent #5153 to at least configure the CI checks to run pyright on 3.10.

@mano7onam
Copy link
Contributor Author

@jakebailey or can be applied to the same type i.e. int | int so the result will be int

@jakebailey
Copy link
Contributor

Sure, my example above isn't complex enough to capture that (I was spitballing), but could have an overload (guessing again):

def __or__(self: _TT, t: _TT) -> _TT: ...

I'm not questioning the construction of it, just whether it needs to be there to not break type checkers.

@east825
Copy link
Contributor

east825 commented Mar 30, 2021

To me, the generic version of the signature looks a bit too complicated, and I'm pretty sure that all type checkers will just special case it similarly to __class_getitem__. Besides, notice that the argument of __or__ is supposed to be a valid type hint, and the current type system doesn't really allow to express that (at least until TypeForm emerges). For instance, it will likely prohibit something like int | Callable[..., Any], because Callable is not bound to type (is it?). Also, since __or__ is defined on type itself, something trivial like int | str will give you Union[Type[int], Type[str]], which is not what it really describes.

I would keep it as simple as the following and let type checkers work out all the magic interpreting the syntax :)

def __or__(self, t: Any) -> Any: ...

@@ -156,6 +157,8 @@ class type(object):
def __subclasscheck__(self, subclass: type) -> bool: ...
@classmethod
def __prepare__(metacls, __name: str, __bases: Tuple[type, ...], **kwds: Any) -> Mapping[str, Any]: ...
if sys.version_info >= (3, 10):
def __or__(self, t: Any) -> types.Union: ...
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def __or__(self, t: Any) -> types.Union: ...
def __or__(self, t: Any) -> Any: ...

As you mentioned, int | int returns int, not a Union. We could do something more clever with overloads, but I'm not sure that's worth it since type checkers will surely special-case this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The type of int | int will still be types.Union
image so we can leave it as it is even for such cases.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I was confused because it gets printed as int, but it's indeed a Union object at runtime. Sorry for the confusion.

Jelles-Air:cpython jelle$ ./python.exe 
Python 3.10.0a7+ (heads/master:28d28e053d, Apr  8 2021, 07:40:29) [Clang 11.0.3 (clang-1103.0.32.59)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> int | int
int
>>> type(_)
<class 'types.Union'>

@JelleZijlstra JelleZijlstra merged commit 31d1c30 into python:master Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants