Opened 3 years ago

Closed 21 months ago

Last modified 12 months ago

#21414 closed Cleanup/optimization (fixed)

Remove django.db.models.related.RelatedObject

Reported by: Anssi Kääriäinen Owned by: Tim Graham
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: loic@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently related fields have two similar attributes, self.rel and self.related. The first is ForeignObjectRel subclass, the second is RelatedObject. Both of these do almost the same thing, and it doesn't seem necessary to have both rel and related. It is confusing to try to remember which one does what.

In the proposed patch at https://github.com/akaariai/django/tree/related_rel_removal the RelatedObject usage is removed. The idea is to make ForeignObjectRel to work exactly like RelatedObject worked and provide the same instance from field.rel and field.related.

I've opted for deprecation path where RelatedObject can still be used, and so can field.related, too. The field.related attribute is actually ForeignObjectRel, but by usage of __instancecheck__ isinstance(field.related, RelatedObject) answers yes. This should make this change easy for 3rd party apps.

Change History (12)

comment:1 Changed 3 years ago by loic84

Cc: loic@… added
Has patch: set
Needs documentation: set
Needs tests: unset
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Version: 1.6master

Looking pretty good.

I left a couple of comments on commit 8d63a8e (linking to it since these are lost on rebase).

Dunno if it's an omission or still on the todo list, but it's missing the deprecation docs.

comment:2 Changed 3 years ago by Anssi Kääriäinen

I am no sure about docs. This is completely private API...

comment:3 Changed 3 years ago by loic84

By docs I meant docs/internals/deprecation.txt, dunno if this document is meant for public consumption or for internal housekeeping.

comment:4 Changed 3 years ago by Aymeric Augustin

It's only for public APIs.

If we know that a private API is used in the wild, we might mention it there, but that's an exception.

comment:5 Changed 3 years ago by loic84

Needs documentation: unset

All good, I wrongly assumed it was a TODO list for when the new dev branch is created.

So yes, it's not needed here since this is very much private APIs.

comment:6 Changed 22 months ago by Tim Graham

Anssi, do you think there is much work to do to incorporate this in 1.8? Might it simply the _meta refactor or should we try to merge that branch first?

comment:7 Changed 22 months ago by Tim Graham

Owner: changed from nobody to Tim Graham
Status: newassigned

I'm trying to update this to apply cleanly.

comment:8 Changed 22 months ago by Tim Graham

Patch needs improvement: unset

comment:9 Changed 21 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In f233bf47dde1d481108142c8d6b4bb3b3d8c6d08:

Fixed #21414 -- Removed RelatedObject and deprecated Field.related.

comment:10 Changed 21 months ago by Phoebe Bright

Found two apps failing as they are using RelatedObject, django-taggit and django-import-export.
To fix, would I be right in thinking it is a case of replacing

from django.db.models.fields.related import ForeignObjectRel
with
from django.db.models.related import RelatedObject

and all instances of RelatedObject with ForeignObjectRel
?

comment:11 Changed 21 months ago by Tim Graham

Yes

comment:12 Changed 12 months ago by Tim Graham <timograham@…>

In 96317ad8:

Refs #21414 -- Removed Field.related per deprecation timeline.

Note: See TracTickets for help on using tickets.
Back to Top