-
-
Notifications
You must be signed in to change notification settings - Fork 118
Add TypeAliasType #160
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
Add TypeAliasType #160
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.
One typo, LG otherwise.
Co-authored-by: Sebastian Rittau <[email protected]>
src/typing_extensions.py
Outdated
self.__name__ = name | ||
self.__value__ = value | ||
self.__type_params__ = type_params |
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 PEP specifies that these attributes should be read-only, so shouldn't they be read-only properties here?
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 suppose we could do that, but it feels unnecessarily complicated.
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 don't feel strongly, but it doesn't feel that complicated to me, and I think it would be nice to match CPython on 3.12+ here 🤷♂️
It might also be worth adding an >>> class Foo(typing.TypeAliasType): ...
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: type 'typing.TypeAliasType' is not an acceptable base type |
def test_no_instance_subclassing(self): | ||
with self.assertRaises(TypeError): | ||
class MyAlias(TypeAliasType): | ||
pass |
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.
(Might be worth adding a test like this to python/cpython#103764 as well; I couldn't spot one)
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.
Good point, python/cpython@d4e72a5
def_mod = _caller() | ||
if def_mod != 'typing_extensions': | ||
self.__module__ = def_mod |
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.
In python/cpython#103764, it looks like __module__
is always "typing"
, no matter what module the type alias is defined in:
>>> type T = int | str
>>> T.__module__
'typing'
I like the behavior you have in this PR more, but it's more complicated to implement, and doesn't match the CPython PR currently :)
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.
(Resolved by python/cpython#104550, for future reference)
Co-authored-by: Alex Waygood <[email protected]>
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.
Looks great!
Co-authored-by: Alex Waygood <[email protected]>
Fixes #159