Skip to content
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

Merged
merged 10 commits into from
May 20, 2023
Merged

Add TypeAliasType #160

merged 10 commits into from
May 20, 2023

Conversation

JelleZijlstra
Copy link
Member

Fixes #159

Copy link
Collaborator

@srittau srittau left a 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.

README.md Outdated Show resolved Hide resolved
Co-authored-by: Sebastian Rittau <srittau@rittau.biz>
Comment on lines 2649 to 2651
self.__name__ = name
self.__value__ = value
self.__type_params__ = type_params
Copy link
Member

@AlexWaygood AlexWaygood May 15, 2023

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?

Copy link
Member Author

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.

Copy link
Member

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 🤷‍♂️

@AlexWaygood
Copy link
Member

It might also be worth adding an __init_subclass__ method to prevent subclassing, as it looks like you get this behaviour with python/cpython#103764:

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

Comment on lines +4497 to +4500
def test_no_instance_subclassing(self):
with self.assertRaises(TypeError):
class MyAlias(TypeAliasType):
pass
Copy link
Member

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)

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines +2660 to +2662
def_mod = _caller()
if def_mod != 'typing_extensions':
self.__module__ = def_mod
Copy link
Member

@AlexWaygood AlexWaygood May 15, 2023

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

Copy link
Member

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)

Copy link
Member

@AlexWaygood AlexWaygood left a 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 <Alex.Waygood@Gmail.com>
@JelleZijlstra JelleZijlstra merged commit 09c1ed4 into python:main May 20, 2023
@JelleZijlstra JelleZijlstra deleted the typealias branch May 20, 2023 23:30
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.

Backport PEP 695's TypeAliasType
3 participants