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

Declaring classes as pure mixins #246

Open
JukkaL opened this issue Jul 14, 2016 · 9 comments
Open

Declaring classes as pure mixins #246

JukkaL opened this issue Jul 14, 2016 · 9 comments
Labels
topic: feature Discussions about new features for Python's type annotations

Comments

@JukkaL
Copy link
Contributor

JukkaL commented Jul 14, 2016

(This is an extension to #241 and copied from the discussion there.)

What about having a magic class decorator (with no or minimal runtime 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'
@dmoisset
Copy link
Contributor

(copying some items from my comments in #241)

I think that omitting the signatures for one_thing and second_thing in MyMixin goes a bit about the documentation and clarity purposes of static typing (which I understand are the main goals of the
typing initiative in Python).

@JukkaL
Copy link
Contributor Author

JukkaL commented Jul 14, 2016

Agreed that the code is not super clear, as there is no explicit documentation about what the mixin expects. I probably wouldn't use this for new code -- it would mainly be aimed at making it easier to migrate legacy code to static typing.

@ilevkivskyi
Copy link
Member

I like this idea, here are few comments:

  • I propose that classes with this decorator will be automatically recognized as implementation base class wherever it appears, i.e., wrapping in Implementation[...] is not necessary.
  • I propose to name decorator @implementation and call such classes implementation base classes (this is in some sense could be a complementary notion to abstract base classes, that define interfaces). In principle, using @Implementation (with capital letter i) could be even better. But then we should also use Implementation(MyClass) in base class lists as @dmoisset proposes in Allow subclassing without supertyping #241.

@leonardoramirezr
Copy link

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'>)

@stinovlas
Copy link

@leonardon473 This is a nice trick, but it can't work with static type checkers, unfortunately. E. g. mypy throws Unsupported dynamic base class "with_typehint" error.

I usually want to declare a Mixin that is common for classes with some common superclass (let's say Django generic views), so I'd like an api like this:

@mixin(Thing)  # or @baseclass(Thing)
class MyMixin:
    x = 1
    def do_stuff(self) -> int:
        return self.one_thing()  # ok, because it's defined in Thing class
    def do_other_stuff(self) -> int:
        return self.y  # fail, there is no attribute `y` in Thing or MyMixin

class Thing(MyMixin):
    def one_thing(self) -> int:
        return 1
    def second_thing(self, x: int) -> str:
        return self.x  # ok, because `x` is defined in MyMixin

@CarliJoy
Copy link

CarliJoy commented Dec 6, 2020

Actually the idea described by @leonardon473 and @stinovlas
Just a reference here some related Issues from StackOverflow:

Now to my Use Case:
Writing MixIn Classes for an existing code base of an Third Base project (i.e. Django, unittest).
So defining a protocol manually as suggested by MyPy won't work. Doing so, I would miss interface changes of the third party package with the type checker.

So what I actually looking for is a command that generates the Protocol of the third Party library for me.

Doing so I would get type hints i.e. from PyCharm and type checking warning ones the interface of the third party tool changes.

So actually @JukkaL proposal of just ignoring the type checks would not satisfy my use case here.

A MixIn should be defined properly, so that it is clear for which classes that MixIn is intended for.

As an example. Imaging I want to test implementations of a Baseclass with unittest. (The Same Use case would it be with a lot of the Django Mixins)

from typing import Type
from unittest import TestCase

class BaseClass:
    def __init__(self, init: str):
        self.name = init

    def foo(self, bar: int) -> str:
        pass

class A(BaseClass):
    def foo(self, bar: int) -> str:
        return f"aaaa {bar} bbbb"

class B(BaseClass):
    def foo(self, bar: int) -> str:
        return f"bbbbbb {bar} aaaaaa"

@mixin(TestCase)
class TestingBaseMixin:
    TEST_CLASS: Type[BaseClass]

    def setUp(self) -> None:
        self.test_obj = self.TEST_CLASS("my_init_stuff")

    def test_something(self) -> None:
        self.assertIn(" 100 ", self.test_obj.foo(100))

class TestA(TestingBaseMixin, TestCase):
    TEST_CLASS = A

class TestB(TestingBaseMixin, TestCase):
    TEST_CLASS = B

What would I expect from @mixin(TestCase):

  1. When testing the MixIn itself TypeCheckers and TypeHinters (like PyCharm) shall assume that all variables and functions are implemented as if I would have used TestingBaseMixin(TestCase)
  2. When checking any class that is actually based on a MixIn. It should make sure that the required that TestCase is actually part of the MRO. If this is not the there shall be a error given to use.
  3. If it is used to defined just another MixIn, the @mixin(Class) decorator is shall be required. So all MixIns are defined well.

Probably there are some edges cases here. Like what if you plan write a MixIn that can be applied to two different Bases?

Another idea would be to implement something like create_protocol_from_class that creates a protocol from an existing class.

Any suggestions on how to bring this topic (Propper type checks with MixIn from Third Party packages) forward?
Shall this be kept in this Issue or better open another one?

@srittau srittau added the topic: feature Discussions about new features for Python's type annotations label Nov 4, 2021
@medihack
Copy link

Years have passed ... and it is still challenging to get the type hints of mixins correctly (if possible at all). Are there any new approaches to type mixins? The solution @leonardon473 came up with seems to work with pyright, but not with mypy.

@erictraut
Copy link
Collaborator

Since this was originally opened, protocols have been added to the type system. Here's a solution to Jukka's original problem using a protocol with a partial implementation. It enforces that the final class, which includes the mixin as a base class, implements the methods that are assumed by the mixin implementation.

from typing import Protocol

class MyMixin(Protocol):
    def do_stuff(self) -> str:
        self.one_thing()
        return self.second_thing(1)

    # The following methods must be implemented by the final class.
    def one_thing(self) -> None:
        ...

    def second_thing(self, val: int) -> str:
        ...

class Thing(MyMixin):
    def one_thing(self) -> None:
        pass

    def second_thing(self, val: int) -> str:
        return "x"

@CarliJoy
Copy link

Indeed this is a little improvement but I still need to duplicate things, as I have to create a protocol for existing classes.

And I have to manually verify the my protocol is identical to the external class I want to write an Mixin for.

I could do something like

from typing import TYPE_CHECKING, Protocol
from extralib import Extra

class MixinProto(Protocol):
    foo: str

if TYPE_CHECKING:
    _: MixinProto = Extra()  # hopefully the class __init__ allows for that...

But this is rather suboptimal and work intensive.

And I don't see that

It enforces that the final class, which includes the mixin as a base class, implements the methods that are assumed by the mixin implementation.

If the final class inherits our Mixin but doesn't provide the given attribute, there won't be any typing errors given...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: feature Discussions about new features for Python's type annotations
Projects
None yet
Development

No branches or pull requests

9 participants