Skip to content

Allow subclassing without supertyping #241

Open

Description

@dmoisset

This issue comes from python/mypy#1237. I'll try to make a summary of the discussion here

Some one reported an issue with an artificial example (python/mypy#1237 (comment)):

class Foo:
    def factory(self) -> str:
        return 'Hello'

class Bar(Foo):
    def factory(self) -> int:
        return 10

and then with the following stub files (python/mypy#1237 (comment)):

class QPixmap(QPaintDevice):
    def swap(self, other: 'QPixmap') -> None: ...

class QBitmap(QPixmap):
    def swap(self, other: 'QBitmap') -> None: ...

Which mypy currently reports as erroneous: Argument 1 of "swap" incompatible with supertype "QPixmap"

These were initially was argued to be a correct error because they violate Liskov Substitution Principle (the same case by a change of return type, the second is a covariant argument change). The problem in this scenario is that the actual classes are not meant to be substituted (the first example is actually a wrapper for a non-virtual C++ function so they're not substitutable, and the second aren't supposed to be mixed together, just reuse part of the code). (see python/mypy#1237 (comment))

There was also a suggestion that allow covariant args explicitly (in the same way that Eiffel allows it and adds a runtime check) with a decorator

class QBitmap(QPixmap):
    @covariant_args
    def swap(self, other: 'QBitmap') -> None: ...

My proposal instead was add what some dialects of Eiffel call "non-conforming inheritance" ("conforms" is the Eiffel word for what PEP-483 calls "is-consistent-with"). It is essentially a mechanism to use subclassing just as a way to get the implementation from the superclass without creating a subtyping relation between them. See python/mypy#1237 (comment) for a detailed explanation.

The proposal is to haven Implementation in typing so you can write:

class QBitmap(Implementation[QPixmap]):
    def swap(self, other: 'QBitmap') -> None: ...

which just defines a QBitmap class with all the mothods copied from QPixmap, but without making one a subtype of the other. In runtime we can just make Implementation[QPixmap] == QPixmap

Activity

  1. dmoisset commented on Jul 8, 2016

    @dmoisset
    ContributorAuthor

    As a follow-up to the discussion, @gvanrossum suggested using a class decorator. The problem with this approach is that class-level is not the right granularity to decide this, but each individual inheritance relation is. Ina multiple-inheritance context, you could want to inherit the interface of a SuperClassA, and the implementation of SuperClassB. I think that might be quite common with mixins (actually I think most mixins could be used with this form of inheritance)

    Looking deeper into what Eiffel does (but this is a bit of a side-comment, not part of the main proposal), there it was also possible to define a superclass in a way that only implementation inheritance from it was allowed, and that could work well with mixins (and implemented as a class decorator). However you still need to be able to say "I just want the implementation of this superclass" when inheriting, because the writer of the superclass may not have foreseen that you wanted that.

  2. dmoisset commented on Jul 8, 2016

    @dmoisset
    ContributorAuthor

    Also note that this idea makes self types more viable. Things like TotalOrderingMixin where you have

        def __lt__(self: T, other:T): -> bool ...

    can be defined more accurately (currently this is in the stdlib), and with no errors (note that the < operator/ __lt__ method is actually covariant in its argument in Python 3). TotalOrderingMixin is something that would create an error in every case with the current mypy implementation if self types are added as is.

  3. ilevkivskyi commented on Jul 10, 2016

    @ilevkivskyi
    Member

    I think this is a very nice feature. In general, it is good that we separate classes (code reuse) and types (debugging/documentation). If it is added, then I would also mention this feature in PEP483 (theory) as an example of differentiating classes and types.

    Between a decorator and a special class I chose the second one, since as @dmoisset mentioned, it is more flexible and suitable fox mixin use cases. Also, it is very in the spirit of gradual typing, one can mark particular bases as Implementation while still typecheck with respect to other bases.

  4. gvanrossum commented on Jul 10, 2016

    @gvanrossum
    Member
  5. ilevkivskyi commented on Jul 14, 2016

    @ilevkivskyi
    Member

    I think it is better to have a minimalistic solution, rather than try to make a complex proposal that will cover all use cases. Later, more features for most popular use cases could be added.

    Here is a detailed proposal with examples:

    1. At runtime Implementation[MyClass] is MyClass. So that the separation of control between subclass/superclass is as usual. Also, all normal mechanisms work at runtime:
    class Base:
        def __init__(self, number: int) -> None:
            self.number = number
    
    class AnotherBase:
        ...
    
    class Derived(Implementation[Base], AnotherBase):
        def __init__(self) -> None:
            super().__init__(42) # This works since Implementation[Base] is just Base
    1. Use cases for Implementation are situations where a user wants to reuse some code in another class without creating excessive "type contracts". Of course one can use Any or #type: ignore, but Implementation will allow more precise type checking.

    2. The derived class is not considered a subtype of implementation base classes. An explicit cast might be used under user's responsibility. Examples:

    from typing import cast
    
    def func(x: Base) -> None:
        ...
    def another_func(x: AnotherBase) -> None:
        ...
    
    func(Base(42))              # this is OK
    another_func(Derived())     # this is also OK
    func(Derived())             # Marked as error by a typechecker
    func(cast(Base, Derived())) # Passes typecheck
    1. Methods of an implementation base class could be redefined in an incompatible way:
    class Base:
        def method(self, x: int) -> None:
            ...
    
    class AnotherBase:
        def another_method(self, y: int) -> None:
            ...
    
    class Derived(Implementation[Base], AnotherBase):
        def another_method(self, y: str) -> None: ... # This is prohibited by a typechecker
        def method(self, x: str) -> None: ...         # Although, this is OK
    1. As @JukkaL proposed, a typechecker checks that the redefined methods do not "spoil" the already checked code by re-type-checking the bodies of all base class methods as if they were written in the subclass (after translating type variables suitably if the base class is generic).

    I think the re-check should be recursive, i.e. include bases of bases, etc. Of course, this could slow down the type-check, but not the runtime.

    1. A typechecker uses the normal MRO to find the signature of a method. For example, in this code:
    class Base:
        def method(self, x: int) -> None: ...
    
    class AnotherBase:
        def another_method(self, y: int) -> None: ...
    
    class Derived(Implementation[Base], AnotherBase):
        def method(self, x: str) -> None: ...

    a typechecker infers that Derived().method has signature (str) -> None, and Derived().another_method has signature (int) -> None.

    1. Clearly, Implementation could be used only with a proper class. Implementation[Union[t1, t2]], Implementation[Callable[[t1, t2, ...], tr]], etc. rise TypeError.

    If we agree on this, then I will submit PRs for typing.py, PEP484, and PEP483.

  6. JukkaL commented on Jul 14, 2016

    @JukkaL
    Contributor

    This has the problem that a type checker doesn't see inside the implementations of stubbed classes, and you could break some of the base class methods without a type checker having any way of detecting it.

    Maybe implementation inheritance would only be allowed from within your own code somehow. For example, behavior would be undefined if you inherit the implementation of a stdlib or 3rd party library class, unless you include the implementation of the library module in the set of checked files. object would be fine as a base class, of course, but you probably shouldn't override any methods defined in object in an incompatible manner, since this can break a lot of things. This would only be enforced by static checkers, not at runtime.

    Not sure if the latter would be too restrictive. To generalize a bit, it would okay if the implementation base class has library classes in the MRO, but methods defined in those classes can't be overridden arbitrarily.

  7. ilevkivskyi commented on Jul 14, 2016

    @ilevkivskyi
    Member

    @JukkaL This is a good point. We need to prohibit incompatibly overriding methods everywhere except for accessible in the set of checked files. I don't think it is too restrictive. Alternatively, we could allow this with a disclaimer that complete typecheking is not guaranteed in this case.

  8. gvanrossum commented on Jul 14, 2016

    @gvanrossum
    Member
  9. ilevkivskyi commented on Jul 14, 2016

    @ilevkivskyi
    Member

    @gvanrossum Indeed, there is not much could be done. But I don't think this is bad. As with Any or # type: ignore, people who are going to use this feature know what they do. More important is that Implementation provides more precise typechecking, more "granularity", and probably a bit more safety than Any.

  10. JukkaL commented on Jul 14, 2016

    @JukkaL
    Contributor

    The QPixmap example shouldn't be a problem, since it seems to be for a library stub file. We don't verify that stubs are correct anyway, so a stub would be able to use implementation inheritance without restrictions but there are no guarantees that things are safe -- we expect the writer of the stub to reason about whether implementation inheritance is a reasonable thing to use.

    For user code, I think that we can provide a reasonable level of safety if we enforce the restrictions that I mentioned.

  11. JukkaL commented on Jul 14, 2016

    @JukkaL
    Contributor

    I have a related proposal. What about having a magic class decorator (with no runtime or minimal effect) or a base class that declares a class as a "pure mixin"? A pure mixin could only be used for implementation inheritance, and it wouldn't be type checked by itself at all -- it would only be type checked as part of implementation inheritance. I think that this would allow type checkers to check some mixin use cases that are tricky right now with minimal code changes. The alternative would be to use ABCs to declare the things that the mixin expects to be defined elsewhere, but they are harder to use and arguably add boilerplate.

    Example:

    @puremixin
    class MyMixin:
        def do_stuff(self) -> int:
            self.one_thing()  # ok, even though not defined in this class
            return self.second_thing(1)  # also ok
    
    class Thing(Implementation[MyMixin]):
        def one_thing(self) -> None: pass  # ok
        def second_thing(self, x: int) -> str:    # error: return type not compatible with do_stuff
            return 'x'
    
  12. gvanrossum commented on Jul 14, 2016

    @gvanrossum
    Member

    But is that the way mixins are typically used? It would be good to look
    carefully at some examples of mixins (e.g. one of our internal code bases
    uses them frequently).

  13. ilevkivskyi commented on Jul 14, 2016

    @ilevkivskyi
    Member

    @JukkaL How about the following disclaimer to be added?

    """
    Note that implementation inheritance could be used with stubbed extension modules, but there are no guarantees of safety in this case (type checkers could not verify correctness of extension modules). The writer of the stub is expected to reason about whether implementation inheritance is appropriate.

    In user code, implementation inheritance is only allowed with classes defined in the set of checked files. Built-in classes could be used as implementation base classes, but their methods could not be redefined in an incompatible way.
    """

    I like you proposal of @puremixin decorator (maybe we could choose other name). It looks like it is independent of the initial proposal and could be easily added. I propose @gvanrossum to decide whether we need this.

  14. JukkaL commented on Jul 14, 2016

    @JukkaL
    Contributor

    @gvanrossum I've seen mixins like that (that use things that aren't defined anywhere in the mixin class). I'm not sure how common they are.

    @ilevkivskyi Looks good. I'd do some small changes (feel free to edit at will):

    Note that implementation inheritance could be used with stubbed extension modules, but there are no guarantees of safety in this case (type checkers could not verify correctness of extension modules). The writer of the stub is expected to reason about whether implementation inheritance is appropriate.

    ->

    Note that implementation inheritance could be used with stubbed extension modules, but there are no guarantees of safety in this case (type checkers could not verify correctness of extension modules). As a special case, if a stub uses implementation inheritance within the stub, the writer of the stub is expected to reason about whether implementation inheritance is appropriate. Stubs don't contain implementations, so type checker can't verify the correctness of implementation inheritance in stubs.

    Built-in classes could be used ...

    ->

    Built-in and library classes could be used ...

  15. JukkaL commented on Jul 14, 2016

    @JukkaL
    Contributor

    Also, I'm not at all attached to the puremixin name.

  16. 31 remaining items

  17. gvanrossum commented on Apr 8, 2017

    @gvanrossum
    Member

    Ah, that summary is very helpful. I agree that one shouldn't call __init__ manually (and in fact Python always reserves the right to have "undefined" behavior when you define or use dunder names other than documented).

    But of course Type[X] has a bunch of behaviors that Callable[..., X] does not (e.g. it has class methods).

  18. mitar commented on Sep 26, 2017

    @mitar
    Contributor

    I have tried to read through this whole thread and I am maybe missing something, but I think I have a slight variation/use case to share to add to this discussion.

    If you are familiar with sklearn you know that it provides many various machine learning classes. It is great because many classes share the same interface, so it is easy to programmers to use them. The issue is that while method names are the same, arguments they receive vary greatly. If I would like to define types for them in a meaningful way, there is a problem. The base class might be something like:

    class Estimator(Generic[Inputs, Outputs]):
        @abstractmethod
        def fit(self, *, inputs: Inputs, outputs: Outputs) -> None:
            pass
    
        @abstractmethod
        def predict(self, *, inputs: Inputs) -> Outputs:
            pass
    
    class SupervisedEstimator(Estimator[Inputs, Outputs]):
        pass
    
    class UnsupervisedEstimator(Estimator[Inputs, Outputs]):
        @abstractmethod
        def fit(self, *, inputs: Inputs) -> None:
            pass
    
    class GeneratorEstimator(Estimator[List[None], Outputs]):
        @abstractmethod
        def fit(self, *, outputs: Outputs) -> None:
            pass
    
    class LupiSupervisedEstimator(SupervisedEstimator[Inputs, Outputs]):
        @abstractmethod
        def fit(self, *, inputs: Inputs, privileged_inputs: Inputs, outputs: Outputs) -> None:
            pass

    So, somehow the idea is that Estimator defines which methods are available, and general arguments, but that then subclasses can define their own combination of arguments for their methods. And that then subclasses of those have to uphold the Liskov Substitution Principle. But one between Estimator and direct subclasses should not matter, because it should be clear that if you are expecting base Estimator, you might know which methods you can use, and what is semantics of those, but you have to inspect required arguments at runtime for an instance you have.

    It seems this is not possible to express with current Python typing so that mypy would not complain?

  19. JukkaL commented on Sep 26, 2017

    @JukkaL
    Contributor

    @mitar Can you give a more complete example which illustrates the errors generated by mypy?

  20. gvanrossum commented on Sep 26, 2017

    @gvanrossum
    Member

    It seems this is not possible to express with current Python typing so that mypy would not complain?

    I can up with an example that seems to work:

    from typing import *
    T = TypeVar('T')
    
    class Base:
        fit: Callable
    
    class Foo(Base):
        def fit(self, arg1: int) -> Optional[str]:
            pass
    
    class Bar(Foo):
        def fit(self, arg1: float) -> str:
            pass

    Maybe this pattern helps? This seems to be about the best you can do given that you don't want the base class to specify the method signatures.

  21. mitar commented on Oct 12, 2017

    @mitar
    Contributor

    Interesting. I never thought of just specifying Callable. This is very useful.

    My ideal typing semantics would be: this is a method signature for fit with arguments inputs and outputs. If your subclass fit uses these arguments, they have to match base fit types, but they can also be specified as omitted (by not listing them in subclass fit arguments).

    Sadly, Optional is not useful here because it is not that a subclass is saying you can pass an argument or not, but that you have to pass it always, or you cannot pass it ever. (So different subclasses would choose one or the other.)

    Then consumers of those objects could check and say: does this method accepts inputs, if yes, then I know what is the type and semantics of this argument. And same for outputs.

  22. danmou commented on Oct 18, 2019

    @danmou

    I've come across a simple case which hasn't been mentioned yet. Variadic arguments can be overridden without (technically) violating the Liskov principle. For example:

    class A:
        def foo(self, *args: Any, **kwargs: Any) -> int:
            ...
    
    class B(A):
        def foo(self, *args: Any, bar: int, **kwargs: Any) -> int:
            ...

    is (to my understanding) in principle valid, but still gives an incompatible with supertype error with mypy.

    The reason I want to do this is: Suppose a library defines the following classes

    class A:
        def foo(self, *args: Any, **kwargs: Any) -> int:
            raise NotImplementedError
    
    class B:
        def __init__(self, obj: A) -> None:
            self.obj = obj
        def get_foo(self) -> int:
            return obj.foo()

    And user code could be something like the following:

    class MyA(A):
        def foo(self, *args: Any, bar: int, **kwargs: Any) -> int:
            return bar
    
    class MyB(B):
        def __init__(self, obj: MyA) -> None:
            super().__init__(obj)
        def get_foo(self) -> int:
            return obj.foo(bar=2)

    Importantly, MyA and MyB could both be defined in different third-party libraries. The original library just defines the general interface and basic functionality. Should this example be accepted by the type-checker? If not, how could it be type annotated?

  23. JelleZijlstra commented on Oct 18, 2019

    @JelleZijlstra
    Member

    Your example is actually unsafe, because a call to x.foo(bar="string") where x: A would be invalid if x is actually an instance of B.

  24. danmou commented on Oct 20, 2019

    @danmou

    @JelleZijlstra that's true. I guess then a better solution would be using Callable without specifying the arguments in the base class, e.g.:

    class A:
        foo: Callable[..., int]

    But of course this still means mypy won't flag something like A().foo() as invalid.

  25. ziima commented on Oct 22, 2019

    @ziima

    @gvanrossum

    Maybe this pattern helps? This seems to be about the best you can do given that you don't want the base class to specify the method signatures.

    Declaring the methods/attributes in mixins is fine if there are just a few of them, but you don't really want to do that when there are a lot of them, for example a mixins for TestCase.

    Having

    class MyTestMixin:
        assertEqual: Callable
        assertRaises: Callable
        ... # 10 other asserts I used in the mixin

    is not really a way I want to go.

  26. leonardoramirezr commented on Jul 9, 2020

    @leonardoramirezr

    Try with:

    from typing import Type, TYPE_CHECKING, TypeVar
    
    T = TypeVar('T')
    
    
    def with_typehint(baseclass: Type[T]) -> Type[T]:
        """
        Useful function to make mixins with baseclass typehint
    
        ```
        class ReadonlyMixin(with_typehint(BaseAdmin))):
            ...
        ```
        """
        if TYPE_CHECKING:
            return baseclass
        return object

    Example tested in Pyright:

    class ReadOnlyInlineMixin(with_typehint(BaseModelAdmin)):
        def get_readonly_fields(self,
                                request: WSGIRequest,
                                obj: Optional[Model] = None) -> List[str]:
    
            if self.readonly_fields is None:
                readonly_fields = []
            else:
                readonly_fields = self.readonly_fields # self get is typed by baseclass
    
            return self._get_readonly_fields(request, obj) + list(readonly_fields)
    
        def has_change_permission(self,
                                  request: WSGIRequest,
                                  obj: Optional[Model] = None) -> bool:
            return (
                request.method in ['GET', 'HEAD']
                and super().has_change_permission(request, obj) # super is typed by baseclass
            )
    
    >>> ReadOnlyAdminMixin.__mro__
    (<class 'custom.django.admin.mixins.ReadOnlyAdminMixin'>, <class 'object'>)
  27. added
    topic: featureDiscussions about new features for Python's type annotations
    and removed on Nov 4, 2021
  28. minrk commented on Jan 11, 2022

    @minrk

    A pattern I've been struggling with is async subclasses of a concrete base class, following the pattern:

    class Thing:
        def count(self) -> int: ...
        def name(self): -> str: ...
    
    class AsyncThing(Thing):
        def count(self): -> Awaitable[int]: ...
        def name(self): -> Awaitable[str]: ...

    where several methods on the subclass return a wrapper of the same type as the parent (in this case, Awaitable). This feels like it should be easy. It certainly is easy to implement, but I don't understand enough yet. In particular, it seems like Awaitable should be something like a TypeVar, e.g.

    WT = TypeVar("T") # 'wrapper' type, may be Null for no wrapping
    class Thing(Generic[WT]):
        def __init__(self: Thing[Null]): # default: no wrapper
        def count(self) -> WT[int]: ... # actually int
        def name(self): -> WT[str]: ...
    
        def not_wrapped(self): -> int # always int, not wrapped
    
        def derivative_method(self) -> WT[str]:
            "Not overridden in subclass, but should match return type of name"
            return self.name()
     
    class AsyncThing(Thing[Awaitable]):
        def count(self): -> Awaitable[int]: ...
        def name(self): -> Awaitable[str]: ...

    but you can't use getitem on TypeVars, so it seems they can't actually be used in this way. Derivative methods that didn't previously need implementations, now also need empty implementations, just to get the updated types.

    Is there any way to accomplish something like this, or is it better to just forego types altogether if one follows this kind of pattern?

  29. JelleZijlstra commented on Jan 11, 2022

    @JelleZijlstra
    Member

    @minrk that pattern is unsafe, because you're violating the Liskov principle. (If you do thing.count() where thing is of type Thing, you may get an Awaitable if thing is actually an AsyncThing.)

    I'd suggest not using inheritance and making AsyncThing instead an independent class that wraps Thing. You could use __getattr__ in that case to implement it, though for type checking purposes you'd probably need to explicitly implement every method.

    An alternative is to just # type: ignore everything. That's what we do in typeshed for the redis library, which has a similar pattern where the pipeline class inherits from the normal client but wraps everything in a pipeline.

  30. minrk commented on Jan 11, 2022

    @minrk

    you're violating the Liskov principle

    Sorry for misunderstanding, I thought that was the point of this Issue: subclassing without preserving Liskov is valid (and in my experience, hugely valuable).

    I'd suggest not using inheritance and making AsyncThing instead an independent class that wraps Thing

    Thanks for the suggestion. Using a HasA wrapper would be significantly more complex because, as you say, I'd have to provide empty implementations of all methods, just to declare the type, when the only concrete benefit would be satisfying mypy. So far, type: ignore seems to be the best compromise, so I'll go with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    topic: featureDiscussions about new features for Python's type annotations

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions