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.

Sign up for GitHub

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

Move RelatedManager to django.db.models.fields.related_descriptors #1814

Merged
merged 2 commits into from
Nov 3, 2023
Merged

Move RelatedManager to django.db.models.fields.related_descriptors #1814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5aa7f4
Move `RelatedManager` to `django.db.models.fields.related_descriptors`
flaeppe Nov 2, 2023
4a063c7
fixup! Move `RelatedManager` to `django.db.models.fields.related_desc…
flaeppe Nov 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion django-stubs/db/models/fields/related_descriptors.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ from django.db.models.base import Model
from django.db.models.fields import Field
from django.db.models.fields.related import ForeignKey, ManyToManyField, RelatedField
from django.db.models.fields.reverse_related import ManyToManyRel, ManyToOneRel, OneToOneRel
from django.db.models.manager import BaseManager, RelatedManager
from django.db.models.manager import BaseManager
from django.db.models.query import QuerySet
from django.db.models.query_utils import DeferredAttribute
from typing_extensions import Self
Expand Down Expand Up @@ -86,6 +86,19 @@ class ReverseManyToOneDescriptor:
def __get__(self, instance: Model, cls: Any = ...) -> type[RelatedManager[Any]]: ...
def __set__(self, instance: Any, value: Any) -> NoReturn: ...

# Fake class, Django defines 'RelatedManager' inside a function body
class RelatedManager(BaseManager[_M], Generic[_M]):
flaeppe marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also changed the base class from Manager to BaseManager.

I suppose it's more correct (managers usually derive from Manager, but not always)? But this causes several new inspections in my projects. Seems like something that should be mentioned in release notes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think we should export RelatedManager and ManyRelatedManager classes from django-stubs-ext to make their usage more convenient.

Right know if anyone wants to use them in type hints, they have to hide the import behind an if TYPE_CHECKING: condition and use quoting for the type, e.g. blah_set: "RelatedManager[Blah]".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed base to align the 2 classes. But any of them is probably fine here really.

And yes, I agree, we should export them.

related_val: tuple[int, ...]
def add(self, *objs: _M | int, bulk: bool = ...) -> None: ...
async def aadd(self, *objs: _M | int, bulk: bool = ...) -> None: ...
def remove(self, *objs: _M | int, bulk: bool = ...) -> None: ...
async def aremove(self, *objs: _M | int, bulk: bool = ...) -> None: ...
def set(self, objs: QuerySet[_M] | Iterable[_M | int], *, bulk: bool = ..., clear: bool = ...) -> None: ...
async def aset(self, objs: QuerySet[_M] | Iterable[_M | int], *, bulk: bool = ..., clear: bool = ...) -> None: ...
def clear(self) -> None: ...
async def aclear(self) -> None: ...
def __call__(self, *, manager: str) -> RelatedManager[_M]: ...

def create_reverse_many_to_one_manager(
superclass: type[BaseManager[_M]], rel: ManyToOneRel
) -> type[RelatedManager[_M]]: ...
Expand All @@ -111,6 +124,7 @@ class ManyToManyDescriptor(ReverseManyToOneDescriptor, Generic[_M]):
@property
def related_manager_cls(self) -> type[ManyRelatedManager[Any]]: ... # type: ignore[override]

# Fake class, Django defines 'ManyRelatedManager' inside a function body
class ManyRelatedManager(BaseManager[_M], Generic[_M]):
related_val: tuple[int, ...]
def add(self, *objs: _M | int, bulk: bool = ...) -> None: ...
Expand Down
13 changes: 0 additions & 13 deletions django-stubs/db/models/manager.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,19 +143,6 @@ class BaseManager(Generic[_T]):

class Manager(BaseManager[_T]): ...

# Fake to make ManyToMany work
class RelatedManager(Manager[_T]):
related_val: tuple[int, ...]
def add(self, *objs: _T | int, bulk: bool = ...) -> None: ...
async def aadd(self, *objs: _T | int, bulk: bool = ...) -> None: ...
def remove(self, *objs: _T | int, bulk: bool = ...) -> None: ...
async def aremove(self, *objs: _T | int, bulk: bool = ...) -> None: ...
def set(self, objs: QuerySet[_T] | Iterable[_T | int], *, bulk: bool = ..., clear: bool = ...) -> None: ...
async def aset(self, objs: QuerySet[_T] | Iterable[_T | int], *, bulk: bool = ..., clear: bool = ...) -> None: ...
def clear(self) -> None: ...
async def aclear(self) -> None: ...
def __call__(self, *, manager: str) -> RelatedManager[_T]: ...

class ManagerDescriptor:
manager: BaseManager
def __init__(self, manager: BaseManager) -> None: ...
Expand Down
2 changes: 1 addition & 1 deletion mypy_django_plugin/lib/fullnames.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
QUERYSET_CLASS_FULLNAME = "django.db.models.query._QuerySet"
BASE_MANAGER_CLASS_FULLNAME = "django.db.models.manager.BaseManager"
MANAGER_CLASS_FULLNAME = "django.db.models.manager.Manager"
RELATED_MANAGER_CLASS = "django.db.models.manager.RelatedManager"
RELATED_MANAGER_CLASS = "django.db.models.fields.related_descriptors.RelatedManager"

WITH_ANNOTATIONS_FULLNAME = "django_stubs_ext.WithAnnotations"
ANNOTATIONS_FULLNAME = "django_stubs_ext.annotations.Annotations"
Expand Down
2 changes: 2 additions & 0 deletions scripts/stubtest/allowlist.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ django.core.files.storage.default_storage

# 'ManyRelatedManager' does exist and is declared locally, inside a function body
django.db.models.fields.related_descriptors.ManyRelatedManager
# 'RelatedManager' does exist and is declared locally, inside a function body
django.db.models.fields.related_descriptors.RelatedManager

# '<Model>_RelatedManager' entries are plugin generated and these subclasses only exist
# _locally/dynamically_ runtime -- Created via
Expand Down
1 change: 0 additions & 1 deletion scripts/stubtest/allowlist_todo.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1708,7 +1708,6 @@ django.db.models.manager.BaseManager.using
django.db.models.manager.BaseManager.values
django.db.models.manager.BaseManager.values_list
django.db.models.manager.Manager.__slotnames__
django.db.models.manager.RelatedManager
django.db.models.options.Options.base_manager
django.db.models.options.Options.concrete_fields
django.db.models.options.Options.db_returning_fields
Expand Down
40 changes: 20 additions & 20 deletions tests/typecheck/fields/test_related.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
book = Book()
reveal_type(book.publisher) # N: Revealed type is "myapp.models.Publisher"
publisher = Publisher()
reveal_type(publisher.books) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Book]"
reveal_type(publisher.books) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Book]"
installed_apps:
- myapp
files:
Expand Down Expand Up @@ -70,8 +70,8 @@
reveal_type(book.publisher2) # N: Revealed type is "myapp.models.Publisher"

publisher = Publisher()
reveal_type(publisher.books) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Book]"
reveal_type(publisher.books2) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Book]"
reveal_type(publisher.books) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Book]"
reveal_type(publisher.books2) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Book]"
installed_apps:
- myapp
files:
Expand Down Expand Up @@ -137,9 +137,9 @@
from django.db import models
class App(models.Model):
def method(self) -> None:
reveal_type(self.views) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.View]"
reveal_type(self.members) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Member]"
reveal_type(self.sheets) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Sheet]"
reveal_type(self.views) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.View]"
reveal_type(self.members) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Member]"
reveal_type(self.sheets) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Sheet]"
reveal_type(self.profile) # N: Revealed type is "myapp.models.Profile"
class View(models.Model):
app = models.ForeignKey(to=App, related_name='views', on_delete=models.CASCADE)
Expand All @@ -153,7 +153,7 @@
- case: test_circular_dependency_in_imports_with_string_based
main: |
from myapp.models import View
reveal_type(View().app.views) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.View]"
reveal_type(View().app.views) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.View]"
View().app.unknown # E: "App" has no attribute "unknown" [attr-defined]
installed_apps:
- myapp
Expand All @@ -172,13 +172,13 @@
from django.db import models
class App(models.Model):
def method(self) -> None:
reveal_type(self.views) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.View]"
reveal_type(self.views) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.View]"

- case: models_related_managers_work_with_direct_model_inheritance_and_with_inheritance_from_other_model
main: |
from myapp.models import App
reveal_type(App().views) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.View]"
reveal_type(App().views2) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.View2]"
reveal_type(App().views) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.View]"
reveal_type(App().views2) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.View2]"
installed_apps:
- myapp
files:
Expand All @@ -196,7 +196,7 @@
- case: models_imported_inside_init_file_foreign_key
main: |
from myapp2.models import View
reveal_type(View().app.views) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp2.models.View]"
reveal_type(View().app.views) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp2.models.View]"
installed_apps:
- myapp
- myapp2
Expand Down Expand Up @@ -280,11 +280,11 @@
from myapp.models import App, Member
reveal_type(Member().apps) # N: Revealed type is "django.db.models.fields.related_descriptors.ManyRelatedManager[myapp.models.App]"
reveal_type(Member().apps.get()) # N: Revealed type is "myapp.models.App"
reveal_type(App().members) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Member]"
reveal_type(App().members) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Member]"
reveal_type(App().members.get()) # N: Revealed type is "myapp.models.Member"
reveal_type(Member.apps.field) # N: Revealed type is "django.db.models.fields.related.ManyToManyField[Any, myapp.models.Member_apps]"
# XXX the following is not correct:
reveal_type(App.members) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Member]"
reveal_type(App.members) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Member]"
installed_apps:
- myapp
files:
Expand Down Expand Up @@ -372,7 +372,7 @@
- case: if_no_related_name_is_passed_create_default_related_managers
main: |
from myapp.models import Publisher
reveal_type(Publisher().book_set) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Book]"
reveal_type(Publisher().book_set) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Book]"
installed_apps:
- myapp
files:
Expand Down Expand Up @@ -474,7 +474,7 @@

publisher = Publisher()
reveal_type(publisher.books)
reveal_type(publisher.books2) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Book]"
reveal_type(publisher.books2) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Book]"
out: |
main:6: error: "Publisher" has no attribute "books"; maybe "books2"? [attr-defined]
main:6: note: Revealed type is "Any"
Expand Down Expand Up @@ -564,8 +564,8 @@
- case: related_manager_name_defined_by_pattern
main: |
from myapp.models import Publisher
reveal_type(Publisher().books) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Book]"
reveal_type(Publisher().articles) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Article]"
reveal_type(Publisher().books) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Book]"
reveal_type(Publisher().articles) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Article]"
installed_apps:
- myapp
files:
Expand Down Expand Up @@ -620,8 +620,8 @@
reveal_type(Article().registered_by_user) # N: Revealed type is "myapp.models.MyUser"

user = MyUser()
reveal_type(user.book_set) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Book]"
reveal_type(user.article_set) # N: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Article]"
reveal_type(user.book_set) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Book]"
reveal_type(user.article_set) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Article]"
installed_apps:
- myapp
files:
Expand Down Expand Up @@ -736,7 +736,7 @@
transaction = models.ForeignKey(Transaction, on_delete=models.CASCADE)
out: |
myapp/models:11: error: Could not resolve manager type for "myapp.models.Transaction.objects" [django-manager-missing]
myapp/models:13: note: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.TransactionLog]"
myapp/models:13: note: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.TransactionLog]"
myapp/models:15: note: Revealed type is "myapp.models.UnknownManager[myapp.models.Transaction]"
myapp/models:16: note: Revealed type is "Any"

Expand Down
4 changes: 2 additions & 2 deletions tests/typecheck/models/test_related_fields.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@

reveal_type(Model2.model_1.field) # N: Revealed type is "django.db.models.fields.related.ForeignObject[app1.models.Model1, app1.models.Model1]"
reveal_type(Model2().model_1) # N: Revealed type is "app1.models.Model1"
reveal_type(Model1.model_2s) # N: Revealed type is "django.db.models.manager.RelatedManager[app1.models.Model2]"
reveal_type(Model1().model_2s) # N: Revealed type is "django.db.models.manager.RelatedManager[app1.models.Model2]"
reveal_type(Model1.model_2s) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[app1.models.Model2]"
reveal_type(Model1().model_2s) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[app1.models.Model2]"

installed_apps:
- app1
Expand Down