Refactor: What is the type-safe way to overload methods with instance arguments, e.g. method(self, other)?

Question

I would like to refactor the below to something type safe. What I have now gives a mypy "incompatible with supertype" error.

I understand this is due to the Liskov substitution principle:

Contravariance of method arguments in the subtype.
Covariance of return types in the subtype.

That is (if I'm understanding correctly), I can return A or a subtype of A, but I can only pass A or a supertype of A (neither of which would have a b attribute) to B.add.

So, I "can't" do what I've been doing, and I'm looking for a way to refactor (more below code).

# python 3.7

class A:
    def __init__(self, a: int) -> None:
        self.a = a

    def add(self, other: "A") -> "A":
        return type(self)(a=self.a + other.a)


class B(A):
    def __init__(self, a: int, b: int) -> None:
        super().__init__(a=a)
        self.b = b

    def add(self, other: "B") -> "B": # Argument 1 of "add" incompatible with supertype "A"
        return type(self)(a=self.a + other.a, b=self.b + other.b)

The only thing that comes to mind is some parent type of A and B with no add method.

class SuperAB:
    # doesn't add
    pass

class A(SuperAB):
    # has add method
    pass

class B(SuperAB):
    # has add method
    pass

That seems like a mess, but if it's the "Pythonic" thing to do, I'll go with it. I'm just wondering if there's another way (besides # type: ignore).

SOLUTION:

After playing "whack a mole" with various type errors, I got to this with the help of StackOverflow answers:

T = TypeVar("T")

class A(Generic[T]):
    def __init__(self, a: int) -> None:
        self.a = a

    def add(self, other: T) -> "A":
        return type(self)(a=self.a + getattr(other, "a"))


class B(A["B"]):
    def __init__(self, a: int, b: int) -> None:
        super().__init__(a=a)
        self.b = b

    def add(self, other: T) -> "B":
        return type(self)(
            a=self.a + getattr(other, "a"), b=self.b + getattr(other, "b")
        )

Note that I can't do self.a + other.a because I'll see a "T" has no attribute "a" error. The above works, but it feels like the respondents here know more than I do, so I've taken their real advice and refactored.

One piece of advice I've seen enough to know is right is "B should have an A, not be an A." I'll confess that this one is beyond my understanding. B has and int (two actually, B.a and B.b). Those ints will do what ints do, but, in order to B.add(B), I've somehow got to put those ints into another B, and if I want that "somehow" to be polymorphic, I'm right back where I started. I'm obviously missing something fundamental about OOP.

@user2357112 Just an MWE, in reality A is a "point" class in a half-edges data structure. Points can have dozens of attributes (hardness, location, color, uv vector, surface normal, etc.). When two Points are "added" together, those attributes have to be reconciled some way. Different Point subclasses have different attributes so need different "add" methods. There are a few other things Points can do (thus the inheritance), but those aren't related to attributes. — Shay, Nov 16 '18 at 18:03
those might need a `has-a` instead of an `is-a` relationship ... — Joran Beasley, Nov 16 '18 at 18:06
possible duplicate: https://stackoverflow.com/questions/46092104/subclass-in-type-hinting ? — Joran Beasley, Nov 16 '18 at 18:06
@JoranBeasley, I don't believe. The problem in that question can be overcome with a more permissive TypeVar. The error here is (wanting to confirm this) warning me of an actual bad practice. — Shay, Nov 16 '18 at 18:08
I dont understand your question ... I copied your classes and have no error ... https://repl.it/@JoranBeasley/ResponsibleIndelibleCharacter ... so either there is no error or your question is lacking critical details — Joran Beasley, Nov 16 '18 at 18:10
@JoranBeasley, are you using MyPy? It's a type-hinting error — Shay, Nov 16 '18 at 18:12
otherwise mypy is just saying that it fails static type checking ... which implies that you need a more permisive type ... something like `def add(self, other: Type["A"]) -> Type["A"]` (maybe ... I really havent messed with type inheritance like that) — Joran Beasley, Nov 16 '18 at 18:12

user2357112 · Accepted Answer · 2018-11-16T18:29:09.600

I probably wouldn't have B inherit from A. That said, for cases where B should inherit from A, you're probably looking for the kind of pattern you see in things like Java's Comparable, where a class takes a subclass as a type parameter:

from abc import ABCMeta, abstractmethod
from typing import Generic, TypeVar

T = TypeVar('T')

class A(Generic[T], metaclass=ABCMeta):
    @abstractmethod
    def add(self, other: T) -> T:
        ...

class B(A['B']):
    def add(self, other: 'B') -> 'B':
        ...

Note that I've marked A abstract, with add as an abstract method. In this kind of situation, it doesn't make much sense for the root declaration of add to be a concrete method, or for one class with a concrete add to subclass another class with a concrete add.

score 1 · Answer 2 · answered Nov 16 '18 at 21:38

If you want to keep your existing class hierarchy, you should probably modify your B's add method so that it behaves reasonably whenever it accepts some instance of A. For example, maybe do the following:

from typing import overload, Union

class A:
    def __init__(self, a: int) -> None:
        self.a = a

    def add(self, other: A) -> A:
        return A(self.a + other.a)


class B(A):
    def __init__(self, a: int, b: int) -> None:
        super().__init__(a=a)
        self.b = b

    # Use overloads so we can return a more precise type when the
    # argument is B instead of `Union[A, B]`.
    @overload
    def add(self, other: B) -> B: ...
    @overload
    def add(self, other: A) -> A: ...

    def add(self, other: A) -> Union[A, B]:
        if isinstance(other, B):
            return B(self.a + other.a, self.b + other.b)
        else:
            return A(self.a + other.a)

b1 = B(1, 2)
b2 = B(3, 4)
a = A(5)

reveal_type(b1.add(b2))  # Revealed type is B
reveal_type(b1.add(a))   # Revealed type is A

# You get this for free, even without the overloads/before making
# any of the changes I proposed above.
reveal_type(a.add(b1))   # Revealed type is A

This restores Liskov and so typechecks. It also makes your add methods symmetric, which is probably the correct thing to do from a usability perspective.

If you don't want this sort of behavior (e.g. if a.add(b1) and b1.add(a) are undesirable), you'll probably want to restructure your code and use the approach user2357112 suggested earlier. Rather then having B inherit A, you could make it contain an instance of A and delegate calls to it as necessary.

Refactor: What is the type-safe way to overload methods with instance arguments, e.g. method(self, other)?

2 Answers2

Linked

Related