Wrong check on multiple inheritance

Let's start with a couple of base classes in a classical diamond-inheritance shape, with member conflict explicitly solved:

class A:
    class M:
        pass

class B0(A):
    class M(A.M):
        pass

class B1(A):
    class M(A.M):
        pass

class C(B0,B1):
    class M(B0.M, B1.M):
        pass

All of these in real code are abstract base classes, classes A , B0, B1 all add behaviors of their own; class C is here precisely to provide this version of M compatible with both B0 and B1 and deal with diamond inheritance in a "centralized" way, so all classes deriving from B0 and B1 don't have to duplicate the same version of M over and over. It could, though, also add behavior of its own.

More abstract classes derive from B0 and B1, each adding more behavior:

class D0(B0):
    pass
class D1(B1):
    pass

And then comes concrete class deriving from D0 and D1, deriving from C to get the diamond inheritance solved:

class D(D0,D1,C):
    pass

python has no issue with this, and understands D.M as defined as C.M. mypy on the other hand does not take python's resolution of the problem into account, and compares all bases classes, leading to false positive:

test-mypy-diamond-inheritance.py:22: error: Definition of "M" in base class "B0" is incompatible with definition in base class "B1"  [misc]

To Reproduce

See https://mypy-play.net/?gist=132dcea42366fad6d56d9c150d8e6c60

I don't think this is a false positive. Mypy is pointing out a legitimate type issue here. There are two base classes of D that define the same symbol (M) inconsistently.

I'm surprised that mypy doesn't point out a similar error for class C, since it's overriding B0.M and B1.M in an incompatible way. So, if anything, I think there's a false negative condition in this code.

If you are confident that this type violation is safe in your code, you can use # type: ignore to suppress mypy's error in this case.

@erictraut

@erictraut that would raise 2 issues:

if there would be a problem, why would python succeed in resolving D.M as C.M ?
if the problem was really about the incompatibility between B0.M and B1.M, I could get rid of the error by explicitly specifying D.M myself (which is how we do when there is a real conflict, that makes python error out)

Further more, using # type: ignore without proper targeting of the error can lead to unwanted errors being silenced, and the more specific we can do is # type: ignore[misc], which I find quite unsatisfactory - but yes, it is what I added with the hope of being able to remove them in the future. For now that's 15 occurrences of this silencing of all misc errors on a class statement, and in a place where users of the API (plugin writers) would have to add more in their own plugin code, quite an uncomfortable situation :/

Mypy reports many forms of type violations that do not necessarily generate exceptions at runtime. The question here is not whether D.M can be resolved, but whether any assumptions made by C or B0 or B1 are violated when they are used together as subclasses for D.

In general, when two or more classes are used as subclasses and they declare a symbol of the same name within their respective scopes, the types of those symbols must be the same. Consider the following example:

class ChildA:
    x: str = "hi"

    def method_a(self):
        print(self.x + " there!")

class ChildB:
    x: int = 0

    def method_b(self):
        print(self.x + 1)

class ParentC(ChildA, ChildB): pass

c = ParentC()
c.method_a()
c.method_b() # Crash

This leads to problems because ChildA and ChildB both define a symbol named x, and they both make assumptions about its type. ChildA assumes that x will be a str, and ChildB assumes that x will be an int. These assumptions are fine when these two classes are used independently. However, when the two classes are combined as subclasses in ParentC, only one view of x is resolved, and this violates the assumptions of one of the subclasses. A runtime exception occurs as a result.

Your code is doing something similar in that it's defining the symbol M in multiple base classes in ways that are not type compatible. You happen to know that classes B0 and B1 will work correctly if some other M is resolved (assuming that all M classes are defined in a manner that conforms to some interface contract), but that's a tenuous assumption in general — and not one that a static type checker can or should make. Mypy is correct in generating a type error here, thus alerting you of the potential problem.

The type-safe way to do what you're doing is to define a protocol class that describes the interface that all M classes must implement. A type checker can then validate that every definition of M is functionally compatible across all subclasses.

class MProto(Protocol):
    # Describe the interface that all M classes must provide
    pass

class A:
    class _M_A: pass
    M: type[MProto] = _M_A

class B0(A):
    class _M_B0(A._M_A): pass
    M: type[MProto] = _M_B0

class B1(A):
    class _M_B1(A._M_A):pass
    M: type[MProto] = _M_B1

class C(B0, B1):
    class _M_C(B0._M_B0, B1._M_B1): pass
    M: type[MProto] = _M_C

class D0(B0): pass
class D1(B1): pass
class D(D0, D1, C): pass

In general, when two or more classes are used as subclasses and they declare a symbol of the same name within their respective scopes, the types of those symbols must be the same.

Why should they be the same ? To me it looks sufficient that any class deriving from both would have a type that would be compatible with the parent's. Provided that we could legally derive from both str and int (whose choice makes that example probably look a bit more outlandish than it should), if we defined in your ParentC class the type of x as deriving from both type(ChildA.x) and type(ChildB.x), it would make both c.method_a() and c.method_b() valid, as it would violate none of their assumptions.

As for using protocols, my feeling right now is that:

it would only help if the protocol would only define interfaces and no default implementation, and to get default implementations to work the protocol class would need to be in the concrete classes' mro, which brings it down to the the status of the simple abc I have right now
it would only work if there was a single protocol, but each M class in my hierarchy adds new attributes and methods, which would make each of them a new protocol: in this example with a single protocol class, mypy would not be able to catch _M_C violating the protocols of _M_B0 and _M_B1. Defining one protocol for every single subclass as I tried here and here would see to 1. violate DRY as each of those protocols just repeats the following class' interface; 2. just hit the same original conflict as my original code; 3. apparently, as I understand mypy's output there, not even accept that the type of M in derived classes would be refined to a subclass. Am I just completely off-track here ?

Ah, I see what you mean now. Yes, I agree with your analysis. I missed the fact that in your example, the resolved version of M in every case is a subtype of all of the "hidden" M symbols, and is therefore guaranteed to be type compatible.

Fix attribute type resolution with multiple inheritance (python#18415)

Navigation Menu

Repository navigation

Wrong check on multiple inheritance #14279

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Wrong check on multiple inheritance #14279

Description

Activity

erictraut commented on Dec 11, 2022

ydirson commented on Dec 11, 2022

erictraut commented on Dec 11, 2022

ydirson commented on Dec 11, 2022

erictraut commented on Dec 11, 2022

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions