Set custom queryset methods as manager attrs instead of method copies

[flaeppe]

Instead of copying methods over from a QuerySet passed to a base manager when invoking <BaseManager>.from_queryset, any QuerySet methods are declared as attributes on the dynamically created manager.

This allows us to properly lookup any QuerySet method types via a get_attribute_hook and will thus remove disorienting phantom errors occuring from mypy trying to resolve types only existing in the module where the original (and real) queryset method was declared.

Consider the currently breaking case of having the following model and queryset modules

# myapp/querysets.py
from django.db import models
from typing import TYPE_CHECKING, Iterable

if TYPE_CHECKING:
    from .models import MyModel

class MyQuerySet(models.QuerySet["MyModel"]):
    def queryset_method(self) -> Iterable[int]:
        return [1]

# myapp/models.py
from django.db import models
from django.db.models.manager import BaseManager
from .querysets import MyQuerySet

NewManager = BaseManager.from_queryset(MyQuerySet)
class MyModel(models.Model):
    objects = NewManager()

Now, to elaborate a bit more, if we expand the NewManager type that django-stubs plugin will dynamically create (without the changes seen in this PR), we get:

# myapp/models.py
from django.db import models
from django.db.models.manager import BaseManager
from .querysets import MyQuerySet

...

class NewManager(BaseManager):
    def queryset_method(self) -> Iterable[int]:
        ...

That happens via copying any method definitions from the passed in queryset (MyQuerySet in this example) on to the manager type. Copying becomes troublesome when queryset and model live in different modules and don't share all definitions. As Iterable isn't imported in myapp/models.py mypy yields an error:

error: Name "Iterable" is not defined

Which makes sense.

Now, to the fix included here. Instead of copying the method, we instead declare each queryset method as a class attribute of the new manager type (in this example NewManager). To display the difference, this is what that will look like if we unwrap the dynamically created manager:

# myapp/models.py
from django.db import models
from django.db.models.manager import BaseManager
from .querysets import MyQuerySet

...

class NewManager(BaseManager):
    queryset_method = MyQuerySet.queryset_method

Mypy is able to resolve that attribute properly without any definition errors. And from my (very shallow) investigation of Django's .from_queryset(more exactly the 3-arg type call) implementation, this representation (if we disregard or unwrap the decorator they implement) aligns better with the truth (feel free to correct me if I'm wrong).

Related issues

Refs #709 (Not sure the fix fully resolves this issue)
Fixes #709 (comment)
Refs #738
Supersedes #802

---

There was a bit of a chain reaction implementing this change, mainly in regards to reversed managers. I'm having a bit of an expectation that there's some edits there that could allow the dynamically created <Model>_<RelatedModel>_RelatedManager named manager types to be replaced with a proper traversal via RelatedModel._default_manager.queryset_method(allowing us to ditch that type creation). But I've tried to leave reversed managers with as few edits as possible and dig in to that thought another time.

[sobolevn]

This is amazing! Idea and execution! Thank you 👏

[sobolevn]

You can add any refactoring you've mentioned in a follow up PRs. I just didn't want this to become stale 🙂

[flaeppe]

You can add any refactoring you've mentioned in a follow up PRs. I just didn't want this to become stale 🙂

Cool, I've opened #821 with those small changes