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

Add django.db.models.fields.related.ForeignObject missing methods and attributes #2154

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add django.db.models.fields.related.ForeignObject missing methods and attributes #2154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3026dc8
Add typing of django.db.models.fields.related.ForeignObject
JaeHyuckSa May 12, 2024
2b6b31a
Merge branch 'master' into improve-related
JaeHyuckSa May 12, 2024
dbd8b3d
Update allowlist_todo.txt
JaeHyuckSa May 12, 2024
c866156
Updated parts that required ClassVar
JaeHyuckSa May 16, 2024
0a91481
Update the __copy__ method
JaeHyuckSa May 16, 2024
33fa5b7
Merge branch 'master' into improve-related
JaeHyuckSa May 16, 2024
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
4 changes: 2 additions & 2 deletions django-stubs/contrib/contenttypes/fields.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from collections.abc import Callable
from typing import Any
from typing import Any, ClassVar

from django.contrib.contenttypes.models import ContentType
from django.core.checks.messages import CheckMessage
Expand Down Expand Up @@ -67,7 +67,7 @@ class GenericRel(ForeignObjectRel):
) -> None: ...

class GenericRelation(ForeignObject):
rel_class: Any
rel_class: ClassVar[Any]
mti_inherited: bool
object_id_field_name: str
content_type_field_name: str
Expand Down
8 changes: 4 additions & 4 deletions django-stubs/db/models/fields/__init__.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,11 @@ class Field(RegisterLookupMixin, Generic[_ST, _GT]):
remote_field: ForeignObjectRel | None
is_relation: bool
related_model: type[Model] | Literal["self"] | None
one_to_many: ClassVar[bool | None]
one_to_one: ClassVar[bool | None]
many_to_many: ClassVar[bool | None]
many_to_one: ClassVar[bool | None]
generated: ClassVar[bool]
one_to_many: bool | None
one_to_one: bool | None
many_to_many: bool | None
many_to_one: bool | None
max_length: int | None
model: type[Model]
name: str
Expand Down
44 changes: 30 additions & 14 deletions django-stubs/db/models/fields/related.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from collections.abc import Callable, Iterable, Sequence
from typing import Any, Generic, Literal, TypeVar, overload
from typing import Any, ClassVar, Generic, Literal, TypeVar, overload
from uuid import UUID

from django.core import validators # due to weird mypy.stubtest error
Expand Down Expand Up @@ -32,14 +32,14 @@ _ST = TypeVar("_ST")
_GT = TypeVar("_GT")

class RelatedField(FieldCacheMixin, Field[_ST, _GT]):
one_to_many: bool
one_to_one: bool
many_to_many: bool
many_to_one: bool
one_to_many: ClassVar[bool]
one_to_one: ClassVar[bool]
many_to_many: ClassVar[bool]
many_to_one: ClassVar[bool]
opts: Any

remote_field: ForeignObjectRel
rel_class: type[ForeignObjectRel]
rel_class: ClassVar[type[ForeignObjectRel]]
swappable: bool
def __init__(
self,
Expand Down Expand Up @@ -87,7 +87,10 @@ class RelatedField(FieldCacheMixin, Field[_ST, _GT]):

class ForeignObject(RelatedField[_ST, _GT]):
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this class has lots of unsolved problems. Let's solve them!

  1. Please, use ClassVar where needed: https://github.com/django/django/blob/a09082a9bec18f8e3ee8c10d473013ec67ffe93b/django/db/models/fields/related.py#L522-L530

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. As you suggested, I have handled all the relevant parts as class variables.

remote_field: ForeignObjectRel
rel_class: type[ForeignObjectRel]
requires_unique_target: ClassVar[bool]
related_accessor_class: ClassVar[type[ReverseManyToOneDescriptor]]
forward_related_accessor_class: ClassVar[type[ForwardManyToOneDescriptor]]
rel_class: ClassVar[type[ForeignObjectRel]]
from_fields: Sequence[str]
to_fields: Sequence[str | None] # None occurs in ForeignKey, where to_field defaults to None
swappable: bool
Expand Down Expand Up @@ -124,6 +127,7 @@ class ForeignObject(RelatedField[_ST, _GT]):
error_messages: _ErrorMessagesMapping | None = ...,
db_comment: str | None = ...,
) -> None: ...
def __copy__(self) -> Self: ...
# class access
@overload
def __get__(self, instance: None, owner: Any) -> ForwardManyToOneDescriptor[Self]: ...
Expand All @@ -142,6 +146,18 @@ class ForeignObject(RelatedField[_ST, _GT]):
def local_related_fields(self) -> tuple[Field, ...]: ...
@cached_property
def foreign_related_fields(self) -> tuple[Field, ...]: ...
def get_local_related_value(self, instance: Model) -> tuple[list[Any], ...]: ...
def get_foreign_related_value(self, instance: Model) -> tuple[list[Any], ...]: ...
@staticmethod
def get_instance_value_for_fields(instance: Model, fields: Sequence[Field]) -> tuple[list[Any], ...]: ...
def get_joining_columns(self, reverse_join: bool = False) -> tuple[tuple[str, str], ...]: ...
def get_reverse_joining_columns(self) -> tuple[tuple[str, str], ...]: ...
def get_extra_descriptor_filter(self, instance: Model) -> dict[str, Any]: ...
def get_path_info(self, filtered_relation: FilteredRelation | None = ...) -> list[PathInfo]: ...
def get_reverse_path_info(self, filtered_relation: FilteredRelation | None = ...) -> list[PathInfo]: ...
@classmethod
def get_class_lookups(cls) -> dict[str, Any]: ...
def contribute_to_related_class(self, cls: type[Model], related: RelatedField) -> None: ...
def get_joining_fields(self, reverse_join: bool = False) -> tuple[tuple[Field, Field], ...]: ...
def get_reverse_joining_fields(self) -> tuple[tuple[Field, Field], ...]: ...

Expand All @@ -150,7 +166,7 @@ class ForeignKey(ForeignObject[_ST, _GT]):
_pyi_private_get_type: Any

remote_field: ManyToOneRel
rel_class: type[ManyToOneRel]
rel_class: ClassVar[type[ManyToOneRel]]
def __init__(
self,
to: type[Model] | str,
Expand Down Expand Up @@ -192,7 +208,7 @@ class OneToOneField(ForeignKey[_ST, _GT]):
_pyi_private_get_type: Any

remote_field: OneToOneRel
rel_class: type[OneToOneRel]
rel_class: ClassVar[type[OneToOneRel]]
def __init__(
self,
to: type[Model] | str,
Expand Down Expand Up @@ -246,13 +262,13 @@ class ManyToManyField(RelatedField[Any, Any], Generic[_To, _Through]):
has_null_arg: bool
swappable: bool

many_to_many: Literal[True]
many_to_one: Literal[False]
one_to_many: Literal[False]
one_to_one: Literal[False]
many_to_many: ClassVar[Literal[True]]
many_to_one: ClassVar[Literal[False]]
one_to_many: ClassVar[Literal[False]]
one_to_one: ClassVar[Literal[False]]

remote_field: ManyToManyRel
rel_class: type[ManyToManyRel]
rel_class: ClassVar[type[ManyToManyRel]]
def __init__(
self,
to: type[_To] | str,
Expand Down
53 changes: 1 addition & 52 deletions scripts/stubtest/allowlist_todo.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,28 +289,13 @@ django.contrib.gis.db.models.FileField.formfield
django.contrib.gis.db.models.FilePathField.formfield
django.contrib.gis.db.models.FloatField.formfield
django.contrib.gis.db.models.ForeignKey.__class_getitem__
django.contrib.gis.db.models.ForeignKey.contribute_to_related_class
django.contrib.gis.db.models.ForeignKey.convert_empty_strings
django.contrib.gis.db.models.ForeignKey.descriptor_class
django.contrib.gis.db.models.ForeignKey.formfield
django.contrib.gis.db.models.ForeignKey.get_db_converters
django.contrib.gis.db.models.ForeignObject.__copy__
django.contrib.gis.db.models.ForeignObject.__get__
django.contrib.gis.db.models.ForeignObject.contribute_to_related_class
django.contrib.gis.db.models.ForeignObject.forward_related_accessor_class
django.contrib.gis.db.models.ForeignObject.get_class_lookups
django.contrib.gis.db.models.ForeignObject.get_extra_descriptor_filter
django.contrib.gis.db.models.ForeignObject.get_extra_restriction
django.contrib.gis.db.models.ForeignObject.get_foreign_related_value
django.contrib.gis.db.models.ForeignObject.get_instance_value_for_fields
django.contrib.gis.db.models.ForeignObject.get_joining_columns
django.contrib.gis.db.models.ForeignObject.get_local_related_value
django.contrib.gis.db.models.ForeignObject.get_path_info
django.contrib.gis.db.models.ForeignObject.get_reverse_joining_columns
django.contrib.gis.db.models.ForeignObject.get_reverse_path_info
django.contrib.gis.db.models.ForeignObject.path_infos
django.contrib.gis.db.models.ForeignObject.related_accessor_class
django.contrib.gis.db.models.ForeignObject.requires_unique_target
django.contrib.gis.db.models.ForeignObject.reverse_path_infos
django.contrib.gis.db.models.ForeignObjectRel.__init__
django.contrib.gis.db.models.ForeignObjectRel.empty_strings_allowed
Expand Down Expand Up @@ -354,8 +339,6 @@ django.contrib.gis.db.models.Model.add_to_class
django.contrib.gis.db.models.ObjectDoesNotExist
django.contrib.gis.db.models.OneToOneField.__get__
django.contrib.gis.db.models.OneToOneField.formfield
django.contrib.gis.db.models.OneToOneField.forward_related_accessor_class
django.contrib.gis.db.models.OneToOneField.related_accessor_class
django.contrib.gis.db.models.OneToOneRel.__init__
django.contrib.gis.db.models.OrderBy.as_oracle
django.contrib.gis.db.models.OrderBy.as_sql
Expand Down Expand Up @@ -724,28 +707,13 @@ django.db.models.FileField.formfield
django.db.models.FilePathField.formfield
django.db.models.FloatField.formfield
django.db.models.ForeignKey.__class_getitem__
django.db.models.ForeignKey.contribute_to_related_class
django.db.models.ForeignKey.convert_empty_strings
django.db.models.ForeignKey.descriptor_class
django.db.models.ForeignKey.formfield
django.db.models.ForeignKey.get_db_converters
django.db.models.ForeignObject.__copy__
django.db.models.ForeignObject.__get__
django.db.models.ForeignObject.contribute_to_related_class
django.db.models.ForeignObject.forward_related_accessor_class
django.db.models.ForeignObject.get_class_lookups
django.db.models.ForeignObject.get_extra_descriptor_filter
django.db.models.ForeignObject.get_extra_restriction
django.db.models.ForeignObject.get_foreign_related_value
django.db.models.ForeignObject.get_instance_value_for_fields
django.db.models.ForeignObject.get_joining_columns
django.db.models.ForeignObject.get_local_related_value
django.db.models.ForeignObject.get_path_info
django.db.models.ForeignObject.get_reverse_joining_columns
django.db.models.ForeignObject.get_reverse_path_info
django.db.models.ForeignObject.path_infos
django.db.models.ForeignObject.related_accessor_class
django.db.models.ForeignObject.requires_unique_target
django.db.models.ForeignObject.reverse_path_infos
django.db.models.ForeignObjectRel.__init__
django.db.models.ForeignObjectRel.empty_strings_allowed
Expand Down Expand Up @@ -787,8 +755,6 @@ django.db.models.Model.add_to_class
django.db.models.ObjectDoesNotExist
django.db.models.OneToOneField.__get__
django.db.models.OneToOneField.formfield
django.db.models.OneToOneField.forward_related_accessor_class
django.db.models.OneToOneField.related_accessor_class
django.db.models.OneToOneRel.__init__
django.db.models.OrderBy.as_oracle
django.db.models.OrderBy.as_sql
Expand Down Expand Up @@ -964,28 +930,13 @@ django.db.models.fields.json.KeyTransform.as_sqlite
django.db.models.fields.json.KeyTransformIsNull.as_sqlite
django.db.models.fields.json.KeyTransformNumericLookupMixin.process_rhs
django.db.models.fields.related.ForeignKey.__class_getitem__
django.db.models.fields.related.ForeignKey.contribute_to_related_class
django.db.models.fields.related.ForeignKey.convert_empty_strings
django.db.models.fields.related.ForeignKey.descriptor_class
django.db.models.fields.related.ForeignKey.formfield
django.db.models.fields.related.ForeignKey.get_db_converters
django.db.models.fields.related.ForeignObject.__copy__
django.db.models.fields.related.ForeignObject.__get__
django.db.models.fields.related.ForeignObject.contribute_to_related_class
django.db.models.fields.related.ForeignObject.forward_related_accessor_class
django.db.models.fields.related.ForeignObject.get_class_lookups
django.db.models.fields.related.ForeignObject.get_extra_descriptor_filter
django.db.models.fields.related.ForeignObject.get_extra_restriction
django.db.models.fields.related.ForeignObject.get_foreign_related_value
django.db.models.fields.related.ForeignObject.get_instance_value_for_fields
django.db.models.fields.related.ForeignObject.get_joining_columns
django.db.models.fields.related.ForeignObject.get_local_related_value
django.db.models.fields.related.ForeignObject.get_path_info
django.db.models.fields.related.ForeignObject.get_reverse_joining_columns
django.db.models.fields.related.ForeignObject.get_reverse_path_info
django.db.models.fields.related.ForeignObject.path_infos
django.db.models.fields.related.ForeignObject.related_accessor_class
django.db.models.fields.related.ForeignObject.requires_unique_target
django.db.models.fields.related.ForeignObject.get_extra_restriction
Copy link
Member

Choose a reason for hiding this comment

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

  1. Let's add other missing attributes to ForeignObject

Copy link
Contributor Author

@JaeHyuckSa JaeHyuckSa May 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have two questions:

  1. I tried to define the get_extra_restriction method, but I encountered an error due to the parameter mismatch with the subclass. I attempted to use type: ignore on the method in the subclass, but it was not allowed. What steps can I take to resolve this issue?
error: not checking stubs due to mypy build errors:
django-stubs/contrib/contenttypes/fields.pyi:90: error: Signature of "get_extra_restriction" incompatible with supertype "ForeignObject"  [override]
django-stubs/contrib/contenttypes/fields.pyi:90: note:      Superclass:
django-stubs/contrib/contenttypes/fields.pyi:90: note:          def get_extra_restriction(self, alias: str, related_alias: str) -> None
django-stubs/contrib/contenttypes/fields.pyi:90: note:      Subclass:
django-stubs/contrib/contenttypes/fields.pyi:90: note:          def get_extra_restriction(self, where_class: Type[WhereNode], alias: Optional[str], remote_alias: str) -> WhereNode
django-stubs/contrib/contenttypes/fields.pyi:92: error: Unused "type: ignore" comment  [unused-ignore]
  1. Should I also add the cached_property method?

django.db.models.fields.related.ForeignObject.reverse_path_infos
django.db.models.fields.related.ForeignObjectRel.__init__
django.db.models.fields.related.ForeignObjectRel.empty_strings_allowed
Expand All @@ -1008,8 +959,6 @@ django.db.models.fields.related.ManyToOneRel.__init__
django.db.models.fields.related.ManyToOneRel.identity
django.db.models.fields.related.OneToOneField.__get__
django.db.models.fields.related.OneToOneField.formfield
django.db.models.fields.related.OneToOneField.forward_related_accessor_class
django.db.models.fields.related.OneToOneField.related_accessor_class
django.db.models.fields.related.OneToOneRel.__init__
django.db.models.fields.related.RelatedField.formfield
django.db.models.fields.related.lazy_related_operation
Expand Down
Loading