Consider changing _BaseQuerySet to QuerySet

[davidhalter]

Bug report

What's wrong

A Jedi user realized that some things about Django stubs did not add up: davidhalter/jedi#1667.

I realized that the reason for this is that _BaseQuerySet does not exist in Django's code base. This is probably a "hack" to work with proper types so that mypy can deal with Django's dynamic nature.

However this leads to frameworks like Jedi (and probably others) not properly infer types from Python to stubs and back.

How is that should be

I'm not sure what the best solution is for this issue. Maybe we can get rid of _BaseQuerySet entirely by using something like class ValuesQuerySet(QuerySet[T], Collection[_Row]), but I'm not sure that's possible.

Another way forward could be to move most of the methods like filter onto QuerySet and duplicate them on ValuesQuerySet.

The best solution would probably be to make QuerySet[T, Row]. This would mean that QuerySet has two type generics. This is great, because this really maps to what a QuerySet actually is in Django. Of course this has the drawback that everywhere where we have QuerySet[T] now, we have to write it like QuerySet[T, T]. But then again this wouldn't affect much:

$ git grep QuerySet\\[ django-stubs/ | wc -l
18

System information

• OS: Ubuntu/Windows/Mac
• python version: 3.6 - 3.10
• django version: 3
• mypy version: -
• django-stubs version: 3d2534e

[sobolevn]

This is a big one. Thanks a lot, @davidhalter, for reporting it!

[syastrov]

QuerySet at one point actually had 2 type parameters (I implemented it in https://github.com/typeddjango/django-stubs/pull/33/files#diff-389fe7d7dcfb77ddaac87c1f3bdc196f3ef0ca33b8047c6638a15f3a3db21b56R53).

I believe it was removed due to concerns that the extra type parameter would scare users (e.g. it would be revealed in error messages), and it is not commonly used.

As you pointed out, you'd have to replace QuerySet[T] with QuerySet[T, Row]. The downside is that (at least on mypy), if you leave out the second type parameter, then mypy (0.790) assumes you just meant QuerySet[Any, Any], which is definitely not what you want. It was possible (at the time I implemented the PR linked above) to implement filling of the second type parameter (as Any) automatically in the mypy plugin, but this is non-standard, and wouldn't work for other type-checkers, or without using the plugin, so I don't think it's desirable.

There was some discussion of making it possible to have defaults for type parameters in Python: python/mypy#4236 but it didn't seem to amount to anything.

Another idea would be to mark the class with decorator @type_check_only (https://docs.python.org/3/library/typing.html#typing.type_check_only). And have jedi somehow skip these classes when going to definitions?

Edit: Possibly combined with this idea, we could make _BaseQuerySet a Protocol as it's not possible to instantiate at runtime anyway. I'm not sure if this alone could help jedi.

[syastrov]

By the way, ValuesQuerySet is also not a valid Django class. At least it was, but was removed in Django 1.9 apparently (https://github.com/django/django/blob/a948d9df394aafded78d72b1daa785a0abfeab48/docs/releases/1.9.txt#L1069)

See also: #144

[davidhalter]

I think if you want to make helpers for users you should probably do something like this:

T = TypeVar("T")
ModelQuerySet = TypeVar("ModelQuerySet", QuerySet[T, Row])
ValuesQuerySet = TypeVar("ValuesQuerySet", QuerySet[T, Dict[str, Any]])

not 100% sure that works in Python. In Rust I would write this:

type ModelQuerySet<T> = QuerySet<T, Row>;
...

Or you could maybe also handle this case with a mypy plugin (not sure that's possible though). However I think the way it's currently typed is just wrong and incomplete. I would prefer the solution I just proposed, but I also understand that this is something pretty unfortunate, especially because values_list(named=True) and values_list(flat=True) exist. This was basically just bad design by Django.

[Feuermurmel]

@davidhalter I think ValuesQuerySet has been added to django_stubs_ext (separate package on PyPI) doing exactly what you proposed:

django-stubs/ext/django_stubs_ext/aliases.py

Line 9 in 0a006f2

ValuesQuerySet = _QuerySet[_T, _Row]

So maybe this issue is obsolete?

Maybe ValuesQuerySet could be made more discoverable by mentioning it somewhere in the readme?

[davidhalter]

@Feuermurmel Good point. I'm not sure about that, because the alias QuerySet: TypeAlias = _QuerySet[_T, _T] would probably still cause issues. The reason for this is that when Jedi tries to look up the non-stub version of _QuerySet.bulk_create, it will fail the lookup, because it obviously doesn't exist in "normal" Django. So as long as the method is not named QuerySet.bulk_create it will probably be hard to look up.

[intgr]

I agree, there are a few hacks involved, including _BaseQuerySet, ValuesQuerySet and QuerySetAny. But I think any possible changes we could make now would have large downsides.

Mypy 1.9 added initial support for PEP 696 (TypeVar defaults), which is a step towards fixing these. We should be able to get rid of these QuerySet hacks once mypy implements complete PEP 696 support -- TypeVar defaulting to another TypeVar (https://peps.python.org/pep-0696/#using-another-type-parameter-as-default)

Related mypy issue:

• Support PEP 696 – Type defaults for TypeVarLikes python/mypy#14851

EDIT: Indeed, full PEP 696 suppot is being worked on.

[intgr]

Update: ValuesQuerySet and QuerySetAny have now been deprecated in #2104, #2194.

I have not looked into _BaseQuerySet yet.

[flaeppe]

I think _QuerySet was named _BaseQuerySet when this issue was reported. Which means that this issue is resolved by #2104 as there's only a QuerySet now.