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

Over-specific return type required for ListView.get_queryset overrides #477

Closed
vgel opened this issue Oct 2, 2020 · 2 comments
Closed
Labels
bug Something isn't working

Comments

@vgel
Copy link

vgel commented Oct 2, 2020

Bug report

What's wrong

Right now django-stubs requires that the return type of a views.generic.ListView subclass' get_queryset method be a queryset, which seems reasonable. However, django doesn't actually require that, and this bit me while adding typing to a project that uses a non-queryset iterable in a view.

See here:
https://github.com/django/django/blob/f87b0ecd37e64e7a019d472de37d0789a8790f1f/django/views/generic/list.py#L21

The return value must be an iterable and may be an instance of QuerySet in which case QuerySet specific behavior will be enabled. (emphasis mine)

V.s. here:

def get_queryset(self) -> QuerySet: ...

Repro case:

from typing import Iterable, Any
from django.views import generic

class TestcaseView(generic.ListView):
    def get_queryset(self) -> Iterable[Any]:
        return [1, 2, 3]

Message:

Return type "Iterable[Any]" of "get_queryset" incompatible with return type "QuerySet[Any]" in supertype "MultipleObjectMixin"

This issue might also affect other view methods -- for example it seems DetailView also is required to require a QuerySet right now:

def get_queryset(self) -> models.query.QuerySet: ...
, but I haven't tested whether django agrees with that restriction.

System information

I can submit a pull for this if there's interest.

@vgel vgel added the bug Something isn't working label Oct 2, 2020
@LucidDan
Copy link

PR #897 seems to be aimed at changing this but isn't linked to the issue.

One concern I have - if this is changed to an Iterable[T] or even Union[Iterable[T], QuerySet[T]], suddenly anyone relying on the Django default CBV behaviour, which is that it WILL always be a QuerySet will need to start doing an isinstance() check before using queryset methods, or they'll get type errors; e.g

qs = get_queryset()
ids = qs.values_list("id") if isinstance(QuerySet, qs) else [obj.id for obj in qs]

But if you are just using the standard CBV classes, the isinstance check should be unnecessary, it will always be a queryset.

Is there a better solution?

@flaeppe
Copy link
Member

flaeppe commented Jul 30, 2024

With the changes in #2191 and the argument in #2174 in mind, I will close this as a won't fix. Since it's totally fair that get_queryset returns a QuerySet

@flaeppe flaeppe closed this as not planned Won't fix, can't repro, duplicate, stale Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

3 participants