gh-108394: Fix subclassing annotated generic alias#108403
gh-108394: Fix subclassing annotated generic alias#108403sobolevn wants to merge 2 commits intopython:mainfrom
Conversation
| self.assertEqual(Z.__mro__, (Z, GenericBase, Generic, Mixin, object)) | ||
|
|
||
| # Errors: | ||
| A1 = Annotated[Literal[1], 'meta'] |
There was a problem hiding this comment.
Can we add an error case with a SpecialForm that is not a GenericAlias, like NoReturn or Self?
| i = bases.index(self) | ||
| except ValueError: | ||
| # It might not be there, if `Annotated[]` is used: | ||
| i = 0 |
There was a problem hiding this comment.
I think this fails in cases like:
class C(typing.Annotated[typing.Callable[[], int], 123]):
passC.__bases__ should include Generic. Another similar case that already fails is
class C(typing.Generic[T], typing.Annotated[int, 123]):
passC.__bases__ lost Generic.
There was a problem hiding this comment.
@eltoder, your fist snippet works fine for me with @sobolevn's PR branch checked out:
Running Release|x64 interpreter...
Python 3.13.0a0 (heads/main:0e8b3fc718, Aug 28 2023, 13:48:35) [MSC v.1932 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from typing import *
>>> class C(Callable[[], int]): ...
...
>>> C.__mro__
(<class '__main__.C'>, <class 'collections.abc.Callable'>, <class 'typing.Generic'>, <class 'object'>)
>>> class C(Annotated[Callable[[], int], 123]): ...
...
>>> C.__bases__
(<class 'collections.abc.Callable'>, <class 'typing.Generic'>)
>>> C.__mro__
(<class '__main__.C'>, <class 'collections.abc.Callable'>, <class 'typing.Generic'>, <class 'object'>)Your second case does indeed look buggy, but as you say, the bug already exists on main. This is the behaviour we get on both this PR branch and the main branch:
Python 3.13.0a0 (heads/main:0e8b3fc718, Aug 28 2023, 13:48:35) [MSC v.1932 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from typing import *
>>> T = TypeVar("T")
>>> class C(Generic[T], Annotated[int, 123]): ...
...
>>> C.__bases__
(<class 'int'>,)There was a problem hiding this comment.
Generic and Annotated together is not tested, please open a new issue about it.
There was a problem hiding this comment.
Yes, I think we would need to special-case _AnnotatedAlias in _GenericAlias.__mro_entries__. The cause of the bug is this behaviour:
>>> from typing import *
>>> T = TypeVar("T")
>>> Generic[T].__mro_entries__((Generic[T], Annotated[int, 123]))
()It's a separate change, unrelated to this one.
There was a problem hiding this comment.
@AlexWaygood sorry, off-by-one in the first example. Should be
class B: pass
class C(B, typing.Annotated[typing.Callable[[], int], 123]):
passThe point is that catching exceptions and setting i = 0 introduces subtle bugs.
There was a problem hiding this comment.
The point is that catching exceptions and setting
i = 0introduces subtle bugs.
Indeed, good catch. Minimal repro:
>>> import types
>>> from typing import *
>>> class A: pass
...
>>> types.new_class('B', (A, Callable[[], int])).__mro__
(<class 'abc.B'>, <class '__main__.A'>, <class 'collections.abc.Callable'>, <class 'typing.Generic'>, <class 'object'>)
>>> types.new_class('B', (A, Annotated[Callable[[], int], 123])).__mro__
(<class 'abc.B'>, <class '__main__.A'>, <class 'collections.abc.Callable'>, <class 'object'>)| if isinstance(self.__origin__, (_GenericAlias, GenericAlias)): | ||
| return self.__origin__.__mro_entries__(bases) |
There was a problem hiding this comment.
I think we should delegate to any __mro_entries__ method, if it exists, not just for _GenericAlias and GenericAlias:
| if isinstance(self.__origin__, (_GenericAlias, GenericAlias)): | |
| return self.__origin__.__mro_entries__(bases) | |
| if hasattr(self.__origin__, "__mro_entries__"): | |
| return self.__origin__.__mro_entries__(bases) |
An example of an improvement this change would lead to: currently you get a nice error message if you try subclassing a NewType instance:
>>> from typing import *
>>> UserID = NewType("UserID", int)
>>> class Foo(UserID): pass
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "C:\Users\alexw\coding\cpython\Lib\typing.py", line 3084, in __init_subclass__
raise TypeError(
TypeError: Cannot subclass an instance of NewType. Perhaps you were looking for: `Foo = NewType('Foo', UserID)`But the nice error message goes away if you subclass Annotated[newtype_instance]:
>>> class Foo(Annotated[UserID, 'must be above 100']): pass
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: NewType.__init__() takes 3 positional arguments but 4 were giveIf we make the change I'm suggesting here, we'll just delegate to NewType.__mro__, so we'll still get the nice error message:
>>> class Foo(Annotated[UserID, 'must be above 100']): ...
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "C:\Users\alexw\coding\cpython\Lib\typing.py", line 3084, in __init_subclass__
raise TypeError(
TypeError: Cannot subclass an instance of NewType. Perhaps you were looking for: `Foo = NewType('Foo', UserID)`
I've decided to go in a different direction from the one suggested by @eltoder
Now, we reuse all
__mro__logic from generic aliases.