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

flaeppe · 2023-11-02T09:10:12Z

Aligns with the module RelatedManager is locally declared

Related issues

Closes #1454

Aligns with the module `RelatedManager` is locally declared

intgr

👍 Makes sense to move it to related_descriptors where Django itself defines it.

django-stubs/db/models/fields/related_descriptors.pyi

…riptors`

intgr · 2023-11-07T12:08:34Z

django-stubs/db/models/fields/related_descriptors.pyi

@@ -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]):


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.

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]".

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.

…only classes (#1834) RelatedManager, ManyRelatedManager classes are sometimes useful for type hinting. But these classes don't exist as is at Django runtime, rather they are defined inside function bodies. * When `TYPE_CHECKING`, we re-export django-stubs fake classes. * At runtime, we define these as `Protocol[_T]`. This has the advantage that Python prevents them being used with isinstance(). Usage before: ```python if TYPE_CHECKING: from django.db.models.manager import RelatedManager # before PR #1814 def get_manager() -> "RelatedManager[MyModel]": ... ``` Usage after: ```python from django_stubs_ext.db.models.manager import RelatedManager def get_manager() -> RelatedManager[MyModel]: ... ```

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

d5aa7f4

Aligns with the module `RelatedManager` is locally declared

flaeppe requested a review from intgr November 2, 2023 09:10

intgr approved these changes Nov 2, 2023

View reviewed changes

django-stubs/db/models/fields/related_descriptors.pyi Show resolved Hide resolved

intgr self-assigned this Nov 2, 2023

fixup! Move RelatedManager to `django.db.models.fields.related_desc…

4a063c7

…riptors`

flaeppe merged commit 0bb2155 into typeddjango:master Nov 3, 2023
35 checks passed

flaeppe deleted the fix/related-manager branch November 3, 2023 08:30

intgr reviewed Nov 7, 2023

View reviewed changes

This was referenced Nov 7, 2023

Next release planning (4.2.7) #1828

Closed

django-stubs-ext: Export RelatedManager, ManyRelatedManager stub-only classes #1834

Merged

This was referenced Nov 15, 2023

Replace Manager with BaseManager typeddjango/djangorestframework-stubs#508

Closed

Restore RelatedManager, ManyRelatedManager to inherit from Manager not BaseManager #1843

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

flaeppe commented Nov 2, 2023

intgr left a comment

intgr Nov 7, 2023

intgr Nov 7, 2023

flaeppe Nov 7, 2023

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

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

Conversation

flaeppe commented Nov 2, 2023

Related issues

intgr left a comment

Choose a reason for hiding this comment

intgr Nov 7, 2023

Choose a reason for hiding this comment

intgr Nov 7, 2023

Choose a reason for hiding this comment

flaeppe Nov 7, 2023

Choose a reason for hiding this comment

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

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