-
-
Notifications
You must be signed in to change notification settings - Fork 31.5k
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
gh-94607: Fix subclassing generics #94610
gh-94607: Fix subclassing generics #94610
Conversation
…inner/cpython into fix_generic_subclasses
I prefer more flexible option mentioned by you in the original comment: block Python types. Just add
at the beginning of the loop. Excluding type subclasses is a common practice, because if we use ducktyping and test for some special method or attribute, the type and the instance usually has the same special method or property. Add tests for |
Sure.
The first example is already tested. The second example won't trigger this code path. The fourth example won't trigger this code path either as unions with |
Is it? Currently I get: >>> List[TypeVar][int]
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/serhiy/py/cpython/Lib/typing.py", line 360, in inner
return func(*args, **kwds)
^^^^^^^^^^^^^^^^^^^
File "/home/serhiy/py/cpython/Lib/typing.py", line 1400, in __getitem__
new_args = self._determine_new_args(args)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/serhiy/py/cpython/Lib/typing.py", line 1437, in _determine_new_args
new_arg = substfunc(new_arg_by_param[old_arg])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: TypeVar.__typing_subst__() missing 1 required positional argument: 'arg' It should raise the same error as for
Yes, but it means that the C code needs a similar fix.
There is no I propose to add the following test: for a in List, list:
for b in int, TypeVar, TypeVarTuple, ParamSpec, types.GenericAlias, types.UnionType:
with self.assertRaisesRegex(TypeError, '.* is not a generic class')
a[b][str] It tests both Python and C implementation of var substitution with all public classes which have |
@serhiy-storchaka oh sorry, I completely misunderstood what you wanted me to test in the first message. I understand now. Thank you for the explanation and the code. |
Your test made me realise that the C version of genericalias needs fixing too. |
Hopefully @serhiy-storchaka can approve this? |
Thanks @Fidget-Spinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
Co-authored-by: Serhiy Storchaka <3659035+serhiy-storchaka@users.noreply.github.com> (cherry picked from commit 6442a9d) Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
GH-94704 is a backport of this pull request to the 3.11 branch. |
The breakage is caused by a discrepancy between the old and new behavior of grabbing type parameters. Previously we only allowed
(_GenericAlias, GenericAlias, types.UnionType)
and would extend with the__parameters__
of these three.After commit b6a5d85, our code grabs anything that has
__parameters__
. The problem is that we shouldn't be extracting__parameters__
from bare Python types which subclass a Generic.There are two possible fixes, either restore the restrictive checks or block bare Python types.
I chose the latter because it allows our code to be more flexible.