sbdchd / django-types

:doughnut: Type stubs for Django
MIT License
202 stars 63 forks source link

Support for Manager.from_queryset #58

Open lpil opened 3 years ago

lpil commented 3 years ago

Hello! Thank you for this library, it is fantastically useful.

In our codebase we're using this method to create our managers.

https://docs.djangoproject.com/en/3.2/topics/db/managers/#from-queryset

It creates a new manager subclass at runtime, and as such the type checker doesn't understand it.

Is there a recommended way to type check this?

Alternatively is there a recommended alternative? I was considering trying to convert it to a mix-in using multiple inheritance, but as a new Python programmer I was unsure if that was wise.

Thanks, Louis

sbdchd commented 3 years ago

Oh that's a tricky one to type. Currently the annotations have it typed as returning Any:

https://github.com/sbdchd/django-types/blob/a2de4090d2f8a2f52fa1ca489c2c063b19c659e5/typings/django/db/models/manager.pyi#L22-L25

using the django docs example:

class CustomManager(models.Manager):
    def manager_only_method(self):
        return

class CustomQuerySet(models.QuerySet):
    def manager_and_queryset_method(self):
        return

class MyModel(models.Model):
    objects = CustomManager.from_queryset(CustomQuerySet)()

I think we need the type of from_queryset to be something like:

# in django/db/models/manager file
M = TypeVar("M", bound=Manager)
Q = TypeVar("Q", bound=QuerySet)

class Manager:
    @classmethod
    def from_queryset(cls: M, queryset: type[Q]) -> M[Q]: ...

But I don't think static types in Python support that. Thinking about alternatives:

lpil commented 3 years ago

OK thank you. I'll experiement for a bit and if I find a good solution I'll get back to you.

lpil commented 3 years ago

I've wound up with this approach:

class DocumentQuerySetProtocol(Protocol):
    # Write signatures here for methods that exist on the QuerySet that are used
    # in the mixin
    pass

class DocumentQuerySetMixin(DocumentQuerySetProtocol):
    # Write methods here to be included in both the query set and the manager
    pass

class DocumentQuerySet(QuerySet, DocumentQuerySetMixin):
    pass

class DocumentManager(Manager, DocumentQuerySetMixin):
    pass

class Document(Model):
    objects: DocumentManager = DocumentManager()

It appears to be working, but I'm far from a Python person 🙃

peterfarrell commented 1 year ago

Really interested if there is an update on this issue or if people have found a better way to get this to work.