Skip to content

PEP 604: Add or operator to class type - #5151

Merged
JelleZijlstra merged 6 commits into
python:masterfrom
mano7onam:master
Apr 8, 2021
Merged

JelleZijlstra merged 6 commits into
python:masterfrom
mano7onam:master

Conversation

@mano7onam

Copy link
Copy Markdown
Contributor

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

@jakebailey

jakebailey commented Mar 29, 2021 •

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

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

@jakebailey

Copy link
Copy Markdown
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

east825 commented Mar 30, 2021

Copy link
Copy Markdown
Contributor

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: ...

Comment thread stdlib/builtins.pyi
@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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