-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
__class_getitem__ Unexpectedly Falls Back to the Metaclass #122634
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
Comments
Quoting PEP:
Moreover, https://docs.python.org/3/reference/datamodel.html#class-getitem-versus-getitem goes into details about So: nor docs, nor PEP say that Here's the example algorithm: from inspect import isclass
def subscribe(obj, x):
"""Return the result of the expression 'obj[x]'"""
class_of_obj = type(obj)
# If the class of obj defines __getitem__,
# call class_of_obj.__getitem__(obj, x)
if hasattr(class_of_obj, '__getitem__'):
return class_of_obj.__getitem__(obj, x)
# Else, if obj is a class and defines __class_getitem__,
# call obj.__class_getitem__(x)
elif isclass(obj) and hasattr(obj, '__class_getitem__'):
return obj.__class_getitem__(x)
# Else, raise an exception
else:
raise TypeError(
f"'{class_of_obj.__name__}' object is not subscriptable"
) And your example follows this logic: >>> class Meta(type):
... def __class_getitem__(cls, key):
... return f'meta got {key!r}'
>>> class Ham(metaclass=Meta):
... pass
>>> hasattr(Ham, '__class_getitem__')
True I am pretty sure that since this is so old, many people rely on this behavior anyway. |
Yeah, this part was missed during the original discussion. After thinking about this a bit, I think the |
It feels like a bug to me. We can raise a |
The current behavior is actively problematic but also extremely unlikely to be triggered. Thus I think it's especially unlikely that there would be user-facing issues fixing this. |
FWIW, I wrote those docs a couple of years ago (after
I don't know that I've seen sufficient evidence to agree that this is "actively problematic" (how many people are really affected by this? what's the realistic use case where it crops up?)... but I agree it seems pretty surprising and buggy. And I agree that it's certainly very unlikely to be triggered. I guess I vote for fixing the bug! |
Ok, let's fix it! There are two ways:
And about backports:
What do you think? Which one is better? |
No backporting; we shouldn't change core language semantics in a bugfix release. If anyone needs to branch on this behavior, they should be able to write I would vote for raising a deprecation first. |
Uh oh!
There was an error while loading. Please reload this page.
Bug report
Bug description:
A major point of
__class_getitem__
(PEP 560) is to avoid metaclasses. This implies that the metaclass should never be involved in the mechanism. However, currently (and since the feature landed) we actually do fall back to the metaclass:(example)
Output:
I was expecting the last one to raise
TypeError: type 'object' is not subscriptable
, sinceHam
does not implement__class_getitem__
. The actual outcome is surprising because the metaclass can already define__getitem__
. Falling back to the metaclass__class_getitem__
doesn't make much sense and can be confusing. The metaclass__class_getitem__
should only be used when subscripting the metaclass, not its instances.The PEP doesn't really address the question of a metaclass that defines
__class_getiem__
1 (nor does the documentation as far as I noticed). Overall, it seems like this was simply not noticed nor considered. It's certainly not an obvious case. Regardless, I think we should fix it.The fix would involve skipping the metaclass part. That would be in the implementation for the subscript syntax (
PyObject_GetItem()
in Objects/abstract.c). There's a part where it specially handles the case where a class is being subscripted. In that case it looks up__class_getitem__
on the class. (See gh-4732.) However, currently it usesPyObject_GetOptionalAttr()
, which involves descriptors and the metaclass (the object's type) and the type's__mro__
. Again, the fix is to skip the metaclass part.FWIW,
__init_subclass__
is fairly similar, but it doesn't fall back to the metaclass. Instead, it effectively doesgetattr(super(cls), '__init_subclass__')
. (Seetype_new_init_subclass()
in Objects/typeobject.c.) We should probably do something similar inPyObject_GetItem()
.CC @ilevkivskyi @gvanrossum
CPython versions tested on:
CPython main branch
Operating systems tested on:
No response
Linked PRs
__class_getitem__
on metaclasses #122743Footnotes
The PEP does say
Note that this method is used as a fallback, so if a metaclass defines __getitem__, then that will have the priority.
but that's specifically about falling back tometa.__getitem__
. ↩The text was updated successfully, but these errors were encountered: