__i<magic>__ method incompatible with __<magic>__ of superclass

[Levitanus]

I'm recieving an a error within folowing code:

class A:
    def __add__(self, other) -> int:
        ...
    def __iadd__(self, other) -> 'A':
        ...

class C(A):
    #  error: Return type of "__iadd__" incompatible with "__add__" of supertype "A"
    def __iadd__(self, other) -> 'C':
        ...

I'm not sure that __add__ and __iadd__ methods have to be compatible.
And, if belief to the MyPy in the signature of class A, that not produce the error, MyPy also doesn't think they have to be compatible...

[ilevkivskyi]

There are lots of subtle special-casing in how Python calls __add__/__iadd__/__radd__. There are therefore a couple of restrictions that must be satisfied in order to be type safe. I am not sure if this particular example is indeed unsafe, but I have found this comment where this error is generated:

                    # An inplace operator method such as __iadd__ might not be
                    # always introduced safely if a base class defined __add__.
                    # TODO can't come up with an example where this is
                    #      necessary; now it's "just in case"

Unfortunately, they are not documented, and every time I am asked about this I spend an hour to write an example of unsafe behavior.

@Michael0x2a you probably have the most context about this. Will you have time to write some docs about dunder related additional checks/errors? (Probably the docs should be in https://mypy.readthedocs.io/en/latest/common_issues.html)

[Michael0x2a]

I did some poking around and it looks like this error message was intended to handle the following case:

class Parent:
    def __add__(self, other: object) -> 'Parent':
        return Parent()

class Child(Parent):
    def __iadd__(self, other: object) -> str:
        return "foo"

var: Parent = Child()
var += 3
# What is the type of 'var'?

This program is indeed not typesafe: at runtime, the final type of var will be str, since we're using Child's __iadd__ method. However, at typecheck time, the only type we can infer for var is Parent: var is declared to be of type Parent, the Parent class doesn't have an __iadd__ type, and so mypy falls back to using the __add__ method. This mirrors what would happen at runtime if var were actually an instance of Parent, instead of Child.

The error message correctly flags that Child's __iadd__ is incompatible with Parent in this case.

However, this check doesn't seem to take into account to take into account is if Parent also defines an __iadd__ method: in that case, we would never fall back to using the __add__ method.

So basically, I think @Levitanus's code snippet ought to type-check, unless I'm missing something.

Anyways, I'll submit a PR to enhance this check and also look into improving either our error messages or our docs, as you suggested.

[Michael0x2a]

Hmm, actually, I think @Levitanus's code snippet is actually not typesafe if the child class is allowed to arbitrarily return NotImplemented:

class A:
    def __add__(self, other) -> int: ...
    def __iadd__(self, other) -> 'A': ...

class C(A):
    def __iadd__(self, other) -> 'C':
        if isinstance(other, str):
            return NotImplemented
        return C()

var: A = C()
var += "foobar"
# At runtime, 'var' will be int, but mypy will infer 'A'.

In fact, if we assume any operator method can return NotImplemented at any time, the definition of A just by itself also wouldn't be typesafe:

class A:
    def __add__(self, other) -> int:
        return 3

    def __iadd__(self, other) -> 'A':
        if isinstance(other, int):
            return NotImplemented
        return A()

var = A()
var += 3
# Inferred type is 'A', but runtime type is 'int'?

So now I'm confused -- what even is our policy regarding NotImplemented? Should we assume the user will return that only if the input argument doesn't match the declared type? Or should we assume any operator method can return it for any reason whatsoever?

[Levitanus]

Inferred type is 'A', but runtime type is 'int'?

Sorry, I missed something... Why is runtime type is int? It has to be 'A'.
As I can conclude – simple operators return the type, which 'value' of class brings:

class A:
    value: int
    def __add__(self, other) -> int:
        assert isinstance(other, int)
        return self.value + other

    def __iadd__(self, other) -> 'A':
        if not isinstance(other, int):
            # also, I'm not a pro programmer, but it seemed to me that raise TypeError
            # is common solution at this case
            return NotImplemented
        # here we are making assignement operation.
        # Can be Your case with "quasi immutable new A()"
        return A(self.__add__(other))
        # or
        self.value = self.__add__(other)
        return self

So, my point of view that __iadd__ method does not the work of adding something, but an assignment of addition operation to something of self-type.

P.S.

The error message correctly flags that Child's iadd is incompatible with Parent in this case.

And this point I now understand and conclude))

[Michael0x2a]

@Levitanus -- you should try running the code samples I posted.

In short, if some binary operator method returns NotImplemented, it's a signal meaning that it doesn't know what to do with the incoming type. Python will then try using the "next" operator method that can potentially return a reasonable result.

For example, if a.__add__(b) returns NotImplemented, Python will then (usually) try b.__radd__(a) before giving up.

Here, if a.__iadd__(b) returns NotImplemented, Python will try a.__add__(b) next before giving up.

This is basically why it's important for the behavior of __add__ and __radd__ as well as __add__ and __radd__ to be consistent with each other, to some degree. The problem is that what Python actually does can be more complicated then what I described above, which is why figuring out exactly what mypy ought to be doing can be challenging.

[ilevkivskyi]

@Michael0x2a I think we had a similar discussion in #4985. The conclusion was that:

• Users are allowed to return NotImplemented whenever they want.
• Imposing 100% type-safety in such situation will practically make operator methods unusable.
• The compromise is to support most typical use cases. For example in that issue we decided that "refining" type of other in a subclass should be OK.

In this case we should probably try to allow something like this:

class B:
    def __iadd__(self, other) -> B:
        ...

class C(B):
    def __iadd__(self, other) -> C:
        ...

since it is quite typical pattern.

[ilevkivskyi]

(Anyway, documenting all kinds of unsafe operator overlap checks and their limitations is important independently of what we will decide here.)