-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
bpo-40336: Refactor typing._SpecialForm #19620
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
bpo-40336: Refactor typing._SpecialForm #19620
Conversation
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.
Thanks, just couple optional suggestions (up to you).
isinstance(args[0], str) and | ||
isinstance(args[1], tuple)): | ||
# Close enough. | ||
raise TypeError(f"Cannot subclass {cls!r}") |
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.
It would be good to keep a custom error message, but not important.
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.
You are right, I tested again class A(Any): pass
, and got "__init__() takes 2 positional arguments but 4 were given
" which does not look user friendly.
But the former message was "Cannot subclass <class 'typing._SpecialForm'>
" which does not look correct, because we subclass an instance of _SpecialForm
, not _SpecialForm
itself. It is not possible to fix in __new__
, but we can raise better error in custom __mro_entries__
.
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.
Using __mro_entries__
is a great idea!
Lib/typing.py
Outdated
|
||
def __repr__(self): | ||
return 'typing.' + self._name | ||
|
||
def __reduce__(self): | ||
return self._name | ||
|
||
def __call__(self, *args, **kwds): | ||
raise TypeError(f"Cannot instantiate {self!r}") |
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.
Again, a custom exception message may be helpful, but not important.
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.
Your are right. "Cannot instantiate typing.Any
" looks better than "'_SpecialForm' object is not callable
". Initially I renamed _SpecialForm
to special form
, but later reverted this change.
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.
Perfect! Thanks for fixing this.
# There is no '_type_check' call because arguments to Literal[...] are | ||
# values, not types. | ||
return _GenericAlias(self, parameters) | ||
raise TypeError(f"{self} is not subscriptable") |
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.
Splitting this is really great. I always wanted to do this, but didn't find a way to do this that wouldn't require boilerplate code, it looks like you have found such way.
I'll leave this to Ivan -- I am busy with the PEG parser integration. |
https://bugs.python.org/issue40336