Remove QuerySet alias hacks via PEP 696 TypeVar defaults

[intgr]

I have made things!

The QuerySet class was previously named _QuerySet and had three aliases: QuerySet, QuerySetAny and ValuesQuerySet. These hacks were mainly needed to for the ergonomic single-parameter QuerySet[Model], which expanded into _QuerySet[Model, Model]

Now that mypy 1.10 implements PEP 696 Type Defaults for Type Parameters to a fuller extent (Pyright also supports it), the 2nd type parameter can be a simple TypeVar that defaults to 1st type parameter. All usages of QuerySetAny and ValuesQuerySet can now be replaced with simple QuerySet.

• Bumps typing-extensions requrement to >=4.11.0, since that's the one that adds TypeVar(default=) parameter. https://github.com/python/typing_extensions/blob/main/CHANGELOG.md#release-4110-april-5-2024

TODO:

• Test behavior on mypy 1.9 and earlier
• Update readme

Related issues

• Fixes Error messages refer to private _QuerySet class #681

[intgr]

Reminder for self.

[intgr]

Tested and this works fine, even though query.pyi now uses _Model TypeVar instead of _T in the QuerySet class (_T still exists).

It's best to keep these aliases as is for now. Since the dependency constraint between django-stubs and django-stubs-ext only goes one way, users may be running e.g. django-stubs-ext 5.0.1 and django-stubs 5.0.0 together.

We can simplify these aliases more a few releases down the line.

[sobolevn]

Great! I think we can now (in the next PRs) get rid of WithAnnotations and provide it as the third type var to QuerySet.

[intgr]

After this change, we strictly require mypy >=1.10.0. Fortunately, older mypy versions will fail quite loudly.

[intgr]

Great! I think we can now (in the next PRs) get rid of WithAnnotations and provide it as the third type var to QuerySet.

A 3rd typevar is not a good long-term solution for that. What we really need is Intersection types.

• Tracking issue: Introduce an Intersection python/typing#213

---

The type of Publisher.objects.annotate(num_books=Count("book")) should boil down to:

class NumBooksAnnotation(Protocol):
    num_books: int

QuerySet[Publisher, Publisher & NumBooksAnnotation]

We currently emulate the & that with our own hand-grown Annotations type and TypedDict instead of Protocol.

We can't get rid of WithAnnotations with a QuerySet typevar anyway. It's not needed for just QuerySets, it's also needed when passing around Model instances containing annotations, e.g.

class NumBooksTypedDict(TypedDict):
    num_books: int

def handle_publisher_with_numbooks(
    publisher: WithAnnotations[Publisher, NumBooksTypedDict]  # <-- (!) not a QuerySet
):
    print(publisher.num_books)


for publisher in Publisher.objects.annotate(num_books=Count("book")):
    handle_publisher_with_numbooks(publisher)

[intgr]

Seemed helpful to add a test for these behaviors. But not entirely sure it's worth it.

[intgr]

Anyway this is ready for review/merging from my point of view.

[sobolevn]

Thanks a lot, really great feature!

[sobolevn]

Do we really need this compat alias?

[intgr]

Currently, yes. ext/django_stubs_ext/aliases.py still expects to import from django.db.models.query import _QuerySet, _QuerySetAny. I'll need to think if we change aliases.py without breaking compatibility.

It's really difficult to think about these, because the dependency constraint between django-stubs and django-stubs-ext only goes one way, users may be running e.g. django-stubs-ext 5.0.1 and django-stubs 5.0.0 together.

I wonder if we can make the version constraint strict django-stubs-ext==5.0.0, or will that cause issues for package managers?

[intgr]

btw when adding review comments, please add it on the last relevant line of a change, rather than the first line, this way more of the diff context is visible.

Right now the message is missing the context that it's about _QuerySet and _QuerySetAny.

[intgr]

Here's an idea:

• Prepare django-stubs-ext for QuerySet typevar refactor #2117

We can get rid of _QuerySetAny after the above is merged. But the _QuerySet alias must remain for the time being.

[sobolevn]

Fair enough, thanks again!

[intgr]

Created an "announcement" discussion for this deprecation as well:

• `ValuesQuerySet` and `QuerySetAny` are now deprecated (django-stubs 5.0.1) #2194