Opened 10 years ago

Closed 9 years ago

#24305 closed New feature (fixed)

Allow overriding abstract model fields

Reported by: Marten Kenbeek Owned by: Aron Podrigal
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Marten Kenbeek, Aron Podrigal, ramezashraf@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Discussion on django-developers: ​https://groups.google.com/forum/#!topic/django-developers/6zUfnElOIks

I suggest we allow fields that are only defined on an abstract base class of a model, to be overridden by models directly inheriting the abstract class. As fields on an abstract class are python-only, and don't live in the database yet, they can easily be switched for another field. If you use abstract models from sources outside your control, or if you reuse an abstract model multiple times, this allows for some flexibility without the need to copy the complete abstract class.

Change History (18)

comment:2 by Tim Graham, 10 years ago

Triage Stage: Unreviewed → Accepted

comment:3 by Marten Kenbeek, 10 years ago

Has patch: set

PR: ​https://github.com/django/django/pull/4184

Feedback welcome, especially on the documentation.

Version 0, edited 10 years ago by Marten Kenbeek (next)

comment:4 by Tim Graham, 10 years ago

Patch needs improvement: set

Chatted with Loic briefly and he indicated he didn't think the locking approach is an appropriate implementation. The approach also needs to integrate with the upcoming work on virtual fields, so it may take some time to sort that out.

comment:5 by Aron Podrigal, 9 years ago

Owner: changed from Marten Kenbeek to Aron Podrigal
Status: new → assigned

comment:6 by Tim Graham, 9 years ago

Patch needs improvement: unset

There is a commit here with an alternate approach: ​https://github.com/knbk/django/commit/7ac5b58587ea2a153766d1601965734731609cdf (having difficulty finding what branch it's part of so I could create a pull request).

comment:8 by Tim Graham, 9 years ago

Patch needs improvement: set

Tests are not passing.

comment:9 by Aron Podrigal, 9 years ago

Patch needs improvement: unset

comment:10 by Aron Podrigal, 9 years ago

Cc: Aron Podrigal added

comment:11 by Tim Graham, 9 years ago

Patch needs improvement: set

The patch is missing some tests cases as noted in Loic's review.

comment:12 by Aron Podrigal, 9 years ago

Patch needs improvement: unset

comment:13 by Aron Podrigal, 9 years ago

Triage Stage: Accepted → Ready for checkin

comment:14 by Ramez Issac, 9 years ago

Cc: ramezashraf@… added
Triage Stage: Ready for checkin → Accepted

I believe that the correct ticket flow is a person other then who supplied the patch can mark Ready For Checkin.
Kindly review
​https://docs.djangoproject.com/en/1.8/internals/contributing/new-contributors/#new-contributors-faq
and
​https://docs.djangoproject.com/en/1.8/internals/contributing/triaging-tickets/

Thanks

comment:15 by Tim Graham, 9 years ago

Triage Stage: Accepted → Ready for checkin

I reviewed the patch and asked Aron to mark the patch as such after updating for my comments. :-)

comment:16 by Ramez Issac, 9 years ago

I apologize for any inconvenience then :-)

comment:17 by Tim Graham, 9 years ago

Summary: Allow to override abstract model fields → Allow overriding abstract model fields

comment:18 by Tim Graham <timograham@…>, 9 years ago

Resolution: → fixed
Status: assigned → closed

In 85ef98dc:

Fixed #24305 -- Allowed overriding fields on abstract models.

Fields inherited from abstract base classes may be overridden like
any other Python attribute. Inheriting from multiple models/classes
with the same attribute name will follow the MRO.

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