-
-
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
Fix Generic with ParamSpec not subscriptable with multiple arguments [non-generic-variant] #491
base: main
Are you sure you want to change the base?
Conversation
Note changes here are the combined checks from |
# we do not check the count | ||
if (inspect.isclass(cls) and issubclass(cls, typing.Generic) | ||
and len(cls.__parameters__) == 1 | ||
and isinstance(cls.__parameters__[0], ParamSpec) |
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.
Do we need to check both typing.ParamSpec and typing_extensions.ParamSpec?
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 do not think so. For 3.10 the _TypeVarLikeMeta
__instancecheck__
of typing_extensions.ParamSpec
will check for the typing variant. And as typing_extensions.ParamSpec.__new__
creates a typing.ParamSpec
everything should be covered.
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 think it might be useful to have a test on 3.10+ that uses typing.Concatenate
and typing.ParamSpec
, which are different from the typing_extensions
version.
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.
Tests added & like noted in #489 for Concatenate a patch was necessary.
This PR assures that no TypeError is raised, however there are still multiple limitations for Python <3.11.3
I think some of the 3.10 cases could be a regression; I am still looking into that and will create a separate issue. #489 will fix most of the cases as a follow up of this PR.