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

Certain multiple inheritance using generic alias errors in 3.13 #112903

Closed
rmartin16 opened this issue Dec 9, 2023 · 17 comments
Closed

Certain multiple inheritance using generic alias errors in 3.13 #112903

rmartin16 opened this issue Dec 9, 2023 · 17 comments
Assignees
Labels
3.13 bugs and security fixes release-blocker stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error

Comments

@rmartin16
Copy link

rmartin16 commented Dec 9, 2023

Bug report

Bug description:

Below is a simplification of some classes that create the problem:

import typing

K = typing.TypeVar("K")
V = typing.TypeVar("V")

class BaseMap(typing.Mapping[K, V]): ...

class MutableMap(BaseMap[K, V], typing.MutableMapping[K, V]): ...

class MyMap(typing.Dict[K, V], MutableMap[K, V]): ...

Just entering these definitions in the REPL causes the error.

The stack trace (with info about variable b):

b=__main__.MutableMap[~V]
type(b)=<class 'types.GenericAlias'>

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    class MyMap(typing.Dict[K, V], MutableMap[V]): ...
  File "/home/russell/.pyenv/versions/3.13-dev/lib/python3.13/typing.py", line 1391, in __mro_entries__
    return super().__mro_entries__(bases)
           ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^
  File "/home/russell/.pyenv/versions/3.13-dev/lib/python3.13/typing.py", line 1143, in __mro_entries__
    if isinstance(b, _BaseGenericAlias) or issubclass(b, Generic):
                                           ~~~~~~~~~~^^^^^^^^^^^^
TypeError: issubclass() arg 1 must be a class

This works "fine" before Python 3.13. If I swap the order of typing.Dict[K, V], MutableMap[V], the error no longer presents on 3.13.

I don't understand enough of intersection of these topics to know if this is now an expected error or something is considered wrong. This does seem related to #103369.

CPython versions tested on:

3.8, 3.9, 3.10, 3.11, 3.12, 3.13, CPython main branch

Operating systems tested on:

Linux

Linked PRs

@rmartin16 rmartin16 added the type-bug An unexpected behavior, bug, or error label Dec 9, 2023
@AlexWaygood AlexWaygood added topic-typing stdlib Python modules in the Lib dir 3.13 bugs and security fixes labels Dec 9, 2023
@sobolevn
Copy link
Member

sobolevn commented Dec 9, 2023

Hm. Looks like this can fix it:

        for b in bases[i+1:]:
            if (
                isinstance(b, (_BaseGenericAlias, types.GenericAlias)) 
                or issubclass(b, Generic)
            ):
                break

I will try to experiment with the fix a bit more to find if it is correct / full.

@JelleZijlstra
Copy link
Member

@rmartin16 thanks for the early testing of Python 3.13! It's very helpful.

And thanks @sobolevn for working on a fix.

@AlexWaygood
Copy link
Member

I'm marking this as a release blocker as it's a backwards-incompatible change that's new in Python 3.13 (@Yhg1s, please remove the label if you disagree!)

@encukou
Copy link
Member

encukou commented Feb 6, 2024

I don't see the backwards incompatibility. It's a regression in main, but it seems that a fix is ready to go in the next Alpha.
Can it be merged, or is there something I don't see that would require the RM's input?

@AlexWaygood
Copy link
Member

AlexWaygood commented Feb 7, 2024

I don't see the backwards incompatibility. It's a regression in main, but it seems that a fix is ready to go in the next Alpha. Can it be merged, or is there something I don't see that would require the RM's input?

The question is whether the proposed fix is the correct fix. The proposed fix uses typing._GenericAlias.__mro_entries__ to switch out instances of types.GenericAlias (a class that is unrelated to typing._GenericAlias) in the __bases__ tuple during the creation of a class that has both an instance of typing._GenericAlias and an instance of types.GenericAlias in its __orig_bases__ tuple. That feels very strange to me; I'd prefer for types.GenericAlias.__mro_entries__ to take charge of whether GenericAlias should be appearing in the __bases__ tuple (but I think that would require changes to C code rather than changes to pure Python code -- typing._GenericAlias is a pure-Python class, but types.GenericAlias is written in C).

I definitely feel like we should fix this regression before Python 3.13 is properly released, but it's probably okay to defer it for now? I don't feel qualified to comment on whether or not this should block a release, though (and if so, which release) -- that's surely up to the Release Manager :)

@AlexWaygood
Copy link
Member

AlexWaygood commented Feb 7, 2024

I have pretty low confidence in my opinions on the proposed fix FWIW, so I'd love it if others could chime in with their opinions!

@AlexWaygood
Copy link
Member

@serhiy-storchaka, since the direct cause here was #103369 — do you have any opinions on this issue or the proposed fix?

@encukou
Copy link
Member

encukou commented Feb 8, 2024

What's the best way to get the attention of typing-sig experts?
I can help, but I feel that I'd first need to become a typing expert myself -- a significant investment :)

@AlexWaygood
Copy link
Member

I think all the people with relevant expertise on this area of code are already subscribed to this issue :) I'd say the active core devs who know this code best at this point are probably Jelle, me, Nikita and Serhiy. @carljm might also have some insight, though!

@AlexWaygood
Copy link
Member

AlexWaygood commented Feb 8, 2024

I can help, but I feel that I'd first need to become a typing expert myself -- a significant investment :)

FWIW, this issue doesn't have much to do with the intricacies of static typing (so most typing-sig experts won't have much to say on it), but it has everything to do with the mechanisms we use to achieve the runtime implementation of typing. https://docs.python.org/3/reference/datamodel.html#object.__mro_entries__, and the references listed at the bottom of the docs for that method, would be the relevant reading required to understand what's going on here, if you are interested in helping :)

The difference between typing._GenericAlias and types.GenericAlias comes down to whether a generic alias is created by machinery internal to the typing module or a builtin PEP-585 alias from outside the typing module

@serhiy-storchaka
Copy link
Member

Does it work with dict, collections.abc.Mapping and collections.abc.MutableMapping instead of typing.Dict, typing.Mapping and typing.MutableMapping? Does it work with mixed types?

@serhiy-storchaka
Copy link
Member

It is weird. It fails for b = MutableMap, but issubclass(MutableMap, typing.Generic) does not fail, and isinstance(MutableMap, typing.GenericAlias) returns False, so the proposed fix should not work.

@serhiy-storchaka
Copy link
Member

Ah, it is MutableMap[K, V], not MutableMap.

Interesting, typing.Mapping[K, V]) has type typing._GenericAlias, typing.MutableMapping[K, V]) has type typing._GenericAlias, but BaseMap[K, V] has type types.GenericAlias.

The simpler way to reproduce the issue is:

import typing
T = typing.TypeVar("T")
class A(typing.Iterable[T], list[T]): ...

@serhiy-storchaka
Copy link
Member

Other, even simpler, example:

import typing
class A(typing.Sized, list[int]): ...

@JelleZijlstra
Copy link
Member

Thanks @serhiy-storchaka for the simpler examples. They helped me find that the proposed fix is wrong; I commented on the PR.

I think to fix this issue we need to either revert #103369, or add special handling to issubclass() calls in typing.py so they deal with GenericAlias correctly.

@carljm
Copy link
Member

carljm commented Feb 8, 2024

We have clearly established for a while now that generic aliases are not types; they have __mro_entries__ so they can be inherited, but otherwise they do not pretend to be types. I don't think we can (or should) backtrack on this position; background on it can be found in #89828. Reverting #103369 is just a very small partial rollback of this position, which just leaves us in an inconsistent state.

I do think the issubclass call in _BaseGenericAlias.__mro_entries__() is simply buggy as written, and must be protected by a prior isinstance(b, type) check. I put more details of that analysis into a comment on the PR.

@AlexWaygood
Copy link
Member

Thanks again @rmartin16 for the early testing, thanks @encukou for bumping the issue (which I'd completely forgotten about!), and thanks @carljm for coming up with a fix!

fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this issue Feb 14, 2024
…_() (python#115191)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes release-blocker stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error
Projects
Development

No branches or pull requests

7 participants