-
-
Notifications
You must be signed in to change notification settings - Fork 110
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
Backport NewType
as it exists on py310+
#157
Conversation
def __init_subclass__(cls): | ||
subcls_name = cls.__name__ | ||
raise TypeError( | ||
f"Cannot subclass an instance of NewType. " |
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.
I feel like this would be clearer as
f"Cannot subclass an instance of NewType. " | |
f"Cannot subclass a NewType. " |
as it may not be obvious to users that "an instance of NewType" refers to a type. I suppose this is the same error as CPython though, so up to you whether it's worth changing.
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.
The nice thing about the "an instance of NewType" wording is that it explains why you can't subclass the thing you're trying to subclass (it's an instance, not a class). But maybe the current phrasing is too terse for the explanation to be useful to people who aren't familiar with how NewType works.
|
||
if sys.version_info >= (3, 10): | ||
# PEP 604 methods | ||
# It doesn't make sense to have these methods on Python <3.10 |
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.
We could allow this in theory, but it would probably be more confusing than useful to users to allow |
on only a small subset of 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.
On the other hand:
Python 3.7.16 (default, Dec 26 2022, 20:28:00)
[Clang 14.0.0 (clang-1400.0.29.202)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from typing_extensions import Self
>>> Self | int
typing.Union[typing_extensions.Self, 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.
On the other hand:
Python 3.7.16 (default, Dec 26 2022, 20:28:00) [Clang 14.0.0 (clang-1400.0.29.202)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> from typing_extensions import Self >>> Self | int typing.Union[typing_extensions.Self, int]
Hahaha that seems wrong though :)
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.
I guess there's no harm in implementing the methods on Python <3.10, but it feels really weird doing an ad-hoc backport of PEP 604 based on the classes we happen to be reimplementing in typing_extensions
.
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.
Seems like we don't backport PEP 604 for TypeVar
or ParamSpec
:
Python 3.7.16 (default, Jan 17 2023, 16:06:28) [MSC v.1916 64 bit (AMD64)] :: Anaconda, Inc. on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import typing_extensions
>>> typing_extensions.TypeVar("T") | int
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for |: 'TypeVar' and 'type'
>>> typing_extensions.ParamSpec("P") | int
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for |: 'ParamSpec' and 'type'
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.
Yes, let's leave this as you wrote it. |
works on objects that use typing_extensions._SpecialForm
on 3.7-3.9, which is arguably wrong but changing it would break compatibility, so let's just leave it as is.
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.
Agreed that it's not worth it to remove __(r)or__
for objects where we've already accidentally backported PEP 604!
Fixes #156. The implementation and tests are basically just copied from CPython, with a few tweaks