typeddjango / djangorestframework-stubs

PEP-484 stubs for django-rest-framework
MIT License
443 stars 116 forks source link

protocol for new mixins #302

Open janrito opened 1 year ago

janrito commented 1 year ago

It would be nice if we could use the classes already defined in generics.py as protocols.

For example, if I want to define a new viewset mixin

class TagModelMixin:
    @action(
        detail=True,
        methods=["PUT"],
        url_path="tag/(?P<slug>[^/.]+)",
    )
    def tag(self, request: Request, slug: str, pk: Any = None) -> Response:
        """Tag an instance with a specific tag"""
        instance = self.get_object()  
        instance.tag.add(slug)
        ...

mypy complains that TagModelMixin does not have a get_object attribute

I think the solution is to define a protocol for the api of GenericAPIView - which is sort of already defined in generics.py

Viicos commented 1 year ago

What if you specify self as being a type of a GenericAPIView? e.g. def tag(self: GenericAPIView, request: Request, slug: str, pk: Any = None) -> Response:

iirc this is the recommended way of doing by mypy, although it is with protocols, so not sure it will work as excepted with a real class.

janrito commented 1 year ago

Yeah, I tried that too, but it complains that the Mixin is not a subclass of GenericAPIVew:

"rest_framework.generics.GenericAPIView[Any]" is not a supertype of its class "app.views.TagModelMixin"  [misc]

You can make it work by having TagModelMixin inherit from GenericAPIView. But none of the original mixins – CreateModelMixin, ListModelMixin ... – do. And the view itself is already inheriting from GenericAPIView, so i feel this is quite dirty.

I ended up implementing a basic protocol:

_ModelType = TypeVar("_ModelType", bound=Model)

class GenericViewSetProtocol(Protocol[_ModelType]):
    def get_object(self) -> _ModelType:
        ...

    def get_serializer(self, *args: Any, **kwargs: Any) -> BaseSerializer[_ModelType]:
        ...
Viicos commented 1 year ago

Yes I believe protocols is the only possibility to correctly type mixins. As it is pretty common to use mixins in drf (and django as well), this could be implemented as a helper protocol

PedroPerpetua commented 1 year ago

I personally use this snippet:

"""Type alias for Mixins."""
if TYPE_CHECKING:
    GenericViewMixin = GenericAPIView
    APIViewMixin = APIView
else:
    GenericViewMixin = object
    APIViewMixin = object

And have my Mixins inherit from the corresponding class.