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 missing dunder attributes for TypeAliasType instances #470

Merged
merged 13 commits into from
Sep 26, 2024

Conversation

Daraan
Copy link
Contributor

@Daraan Daraan commented Sep 25, 2024

Adresses: #449

As @JelleZijlstra mentioned that GenericAlias can be used to address #469. GenericAlias seems promising, however I am not sure what greater underlying changes this swap could maybe cause.

Currently I am still tracing some errors and I want to make sure I am not regressing some cases that currently are not covered by tests. This is also a reason this is currently only used for 3.10+.
See below I think using it in 3.9 will cause more harm than benefits.

@Daraan Daraan changed the title Type alias type/dunder attributes Add missing dunder attributes for TypeAliasType instances Sep 25, 2024
@Daraan
Copy link
Contributor Author

Daraan commented Sep 25, 2024

From what I see GenericAlias can only be safely used here for Python3.10+

In 3.9 it will raise an TypeError in belows test as it tries to subscript the _ConcatenateGenericAlias which inherits from list. The location where this is causes is located here https://github.com/python/cpython/blob/8b9a8e0e082f849f6db03b2ed9074a86813b3b77/Lib/typing.py#L779

With _GenericAlias I can get the below test to pass in #449 however with GenericAlias this seems tricky.

    def test_callable_with_concatenate(self):
        P = ParamSpec('P')
        CallableP = TypeAliasType("CallableP", Callable[P, Any], type_params=(P,))

        callable_concat = CallableP[Concatenate[int, P]]
        self.assertEqual(callable_concat.__parameters__, (P,))
        concat_usage = callable_concat[str]  # <-- will fail for 3.9 with GenericAlias

@Daraan Daraan marked this pull request as ready for review September 25, 2024 17:22
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.

Thanks! I pushed a slightly different solution to your PR branch (using a _GenericAlias subclass) so that all the tests would pass on py38 and py39 as well. Your tests were fantastic; they really helped to see all aspects of the problem!

@AlexWaygood AlexWaygood merged commit 832253d into python:main Sep 26, 2024
21 checks passed
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.

3 participants