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

protocol for new mixins #302

Open
janrito opened this issue Nov 28, 2022 · 4 comments
Open

protocol for new mixins #302

janrito opened this issue Nov 28, 2022 · 4 comments

Comments

@janrito
Copy link

janrito commented Nov 28, 2022

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
Copy link
Contributor

Viicos commented Jan 7, 2023

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
Copy link
Author

janrito commented Jan 9, 2023

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
Copy link
Contributor

Viicos commented Jan 9, 2023

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
Copy link

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.

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

No branches or pull requests

3 participants