Self typed custom queryset methods incompatible with base queryset type

[MrkGrgsn]

Bug report

What's wrong

Custom queryset methods with a return type of Self have a return type incompatible with the base QuerySet class.

class LogQuerySet(QuerySet["Log"]):

    def filter_a(self) -> QuerySet["Log"]:
        return self

    def filter_b(self) -> Self:
        return self

LogManager = Manager.from_queryset(LogQuerySet)

class Log(models.Model):
    name = models.CharField(max_length=100)
    objects = LogManager()

a: models.QuerySet[Log] = Log.objects.filter_a()  # OK
b: models.QuerySet[Log] = Log.objects.filter_b()  # Incompatible types in assignment

mypy reports:

Incompatible types in assignment (expression has type "ManagerFromLogQuerySet[Log]", variable has type "_QuerySet[Log, Log]") [assignment]

How is that should be

It would be great if the return type was inferred to be a sub-type of QuerySet with the correct type, eg, QuerySet[Log].

System information

• OS:
• python version: 3.11.6/3.8.18
• django version: 3.2.23
• mypy version: 1.6.1
• django-stubs version: 4.2.6
• django-stubs-ext version: 4.2.5

[MrkGrgsn]

I've been investigating this. It arises from #1789 , fixing #1788, where it was specified that queryset method return types of Self used in from_queryset() or as_manager() should resolve to the dynamic manager class. The issue is that such a method does actually return a queryset and the dynamic manager class is not a queryset (or at least mypy doesn't think so). Referring back to the example in the description of this issue, I think that Self should resolve to LogQuerySet[Log, Log].

[moranabadie]

I do not think this is realy an error. Since Log.objects is not realy a LogQuerySet[Log, Log]. It is a generated manager by Django that derivates from LogQuerySet, but is not a LogQuerySet. So self should not rersolve a
LogQuerySet striclty : it has its methods but also Manager methods for example.

But maybe the issue here is more global (cf @flaeppe) that ManagerFromLogQuerySet is not detected has a subclass of LogQuerySet, which I think it is.

[MrkGrgsn]

Hi @moranabadie, thanks for taking a look. The manager that Django creates in from_queryset or as_manager is conceptually derived from LogQuerySet but not by sub-classing the queryset. Rather the manager has methods created that wrap the queryset methods. It makes sense (to me anyway) that in this scenario Self is resolved with respect to the queryset and not the dynamic manager class.

Perhaps the plugin's current application of Self would work for the example I've provided if ManagerFromLogQuerySet was a sub-class of LogQuerySet. But I don't believe it's correct to say that queryset methods wrapped on the derived manager class return an instance of the derived manager class as managers have methods and attributes that querysets do not.

[flaeppe]

This stems from the fact that Django dynamically adds queryset methods as attributes of a manager. The mypy plugin simulates that behaviour as well. There's a note hinting towards that happening, which is also slightly relevant:

django-stubs/django-stubs/db/models/manager.pyi

Lines 37 to 38 in 82c394f

	# NOTE: The following methods are in common with QuerySet, but note that the use of QuerySet as a return type
	# rather than a self-type (_QS), since Manager's QuerySet-like methods return QuerySets and not Managers.

We probably need to help mypy resolve Self (for convenience, usability and to avoid confusion) in the plugin when constructing the dynamic manager (in this case called ManagerFromLogQuerySet). As mypy resolves to its current self(the manager), even though the method came from a different class.

You would probably also find the incorrectness by printing out the return type of .filter_b() e.g. print(type(Log.objects.filter_b())) and see that it's indeed of LogQuerySet and not ManagerFromLogQuerySet

I happen to know that the plugin strips the first argument (self) when adding queryset methods to the initialized manager (mypy does this too, see reveal for line 13 in playground link below).

Stripping happens here via the [1:]

django-stubs/mypy_django_plugin/transformers/managers.py

Lines 143 to 150 in 82c394f

	# Drop any 'self' argument as our manager is already initialized
	return method_type.copy_modified(
	arg_types=args_types,
	arg_kinds=method_type.arg_kinds[1:],
	arg_names=method_type.arg_names[1:],
	variables=variables,
	ret_type=ret_type,
	)

If we try to simulate the plugin behaviour manually in playground, we see that mypy emits an error for the self argument, which happen to be stripped by the plugin: https://mypy-play.net/?mypy=latest&python=3.11&gist=b2aee3895956ae3e8ceaaefe4837d0ad

Inlining playground contents for clarity

from typing_extensions import Self

class QuerySet:
    def method(self) -> Self:
        return self
    

class Manager:
    method = QuerySet.method
    
    
reveal_type(Manager.method)
reveal_type(Manager().method)
reveal_type(Manager().method())

I'll just keep going here, mostly for notes to self. But the issues emitted from the above snippet can instead of stripping be fixed by e.g. annotate every manager method as:

+from typing import Any, Callable, ClassVar
from typing_extensions import Self

class QuerySet:
    def method(self) -> Self:
        return self
    

class Manager:
-    method = QuerySet.method
+    method: ClassVar[Callable[[Any], QuerySet]] = QuerySet.method
    
    
reveal_type(Manager.method)
reveal_type(Manager().method)
reveal_type(Manager().method())

Notes:

• It's only valid to swap out typing_extensions.Self or typing.Self in the return type
• Instead of stripping the self parameter we could annotate it as Any on the manager callable (as Manager and QuerySet don't share any bases)
• We should make sure all methods are ClassVars (I think?)
• We have to be careful to not annotate types that doesn't exist in the module, in case we add annotations, refs:
  • Custom manager causes phantom errors and sometimes crash #709 (comment)
  • Set custom queryset methods as manager attrs instead of method copies #820 (comment)

[moranabadie]

Hi @moranabadie, thanks for taking a look. The manager that Django creates in from_queryset or as_manager is conceptually derived from LogQuerySet but not by sub-classing the queryset. Rather the manager has methods created that wrap the queryset methods. It makes sense (to me anyway) that in this scenario Self is resolved with respect to the queryset and not the dynamic manager class.

Perhaps the plugin's current application of Self would work for the example I've provided if ManagerFromLogQuerySet was a sub-class of LogQuerySet. But I don't believe it's correct to say that queryset methods wrapped on the derived manager class return an instance of the derived manager class as managers have methods and attributes that querysets do not.

Oups, you seem to be right. I will try to fix it in a pull request