Model Field Order not influenced by MRO of superclasses

[Klaas van Schelven]

Looking at the two definitions below,

class OneTwo(AbstractModelOne, AbstractModelTwo):
    pass

class TwoOne(AbstractModelTwo, AbstractModelOne):
    pass

one (I?) would expect the order of the fields in the second definition to be the opposite of the order in the first one. (Since the base classes are referenced in the opposite order). This is, however, not the case.

Django only uses creation_counter to determine the order of the fields. Therefor the actual field order is determined by the order in which the containing abstract classes are evaluated (i.e. their python code is read). If these classes come from multiple sources (apps) the field order may end up to be pretty much random (though deterministic).

The general use case is heavy use of abstract models to mix in information from various sources into actual models. In real apps the two above models are not very likely, but being able to have control over the field order (other than placing the abstract models in a certain order) is important.
I make heavy use abstract models to be able to make my apps reusable. The above situation is an important roadblock to efficient use.

A patch with a failing test is attached.
Haven't yet been able to fix this in a non-intrusive way.
I would be very much obliged if someone can come up with a solution.

[Klaas van Schelven]

The attached file "failed_attempt" shows an initial stab at the problem. Running the tests quickly shows that this indeed does not solve it (other tests regress); but this is where my understanding of the ORM ends for now.

[Łukasz Rekucki]

One thing that I don't understand is why does field order matter to you ? Maybe your problem can be solved easier on a different level.

[Klaas van Schelven]

Replying to lrekucki:

One thing that I don't understand is why does field order matter to you ? Maybe your problem can be solved easier on a different level.

The field order is mainly important wherever I use ModelForms.
True, I can post-hoc fix everything there, but that doesn't seem very DRY.

[Łukasz Langa]

Required functionality (sans m2m fields) + tests; based on r15176

[Julien Phalip]

I've updated the patch so that it applies to trunk. I've also renamed the 'bases' attribute to 'model_bases' to avoid any confusion.

I'm RFC'ing this patch as I think it solves an annoying inconsistency. I've just got one little reservation: possibly this will be backwards incompatible for people who were relying on that inconsistency. Although I don't think this is a big deal. I'll leave the core devs to make a call on this.

[Russell Keith-Magee]

This patch appears to be headed broadly in the right direction, and it works, but I'm not convinced that it's quite right yet.

In particular, the fact that it is introducing a new model_bases attribute to the Meta class when parents already exists concerns me. Granted, the two aren't completely identical -- parents doesn't include any abstract classes, for example -- but it strikes me that the right solution here isn't just to add a new block of almost identical metadata, but to adjust the existing metadata, and adjust the existing usage.

The Meta class is in bad need of some housekeeping -- I'd rather not knowingly add to that housekeeping work.

[Łukasz Langa]

The reason I introduced model_bases instead of rearranging half the framework around changes in parents was to keep the patch minimal (e.g. easier to get right). On top of that, I believe in backwards compatibility and I'm not sure if there isn't some 3rd party code using parents in ways that could have been broken by changing that.

I am willing to help with any refactoring of Meta necessary. However, this is naturally milestone 1.4, e.g. over 9 months away. Could you please reassess whether we could integrate the patch as it is? Without that change I will need to basically fork Django until another stable release. This is not easy to monkey patch.

[Russell Keith-Magee]

If we were talking about a critical component of Django, I might agree to taking a "fix the problem, then fix the fix" approach. In fact, I did just that with #9964 only days ago. However, I don't think this problem falls into that category. Django has had model inheritance for a while, and this is the first time this problem has been reported. It requires a complex model inheritance structure and a reliance on automated form ordering before it becomes a problem.

I'm sorry that deferring this will be inconvenient to you, but I'd ask you to also consider *my* inconvenience -- the fact that landing a suboptimal bugfix *increases* my work as a framework maintainer in the long run, because at some point, I'll have to reconcile two disparate representations of base classes.

Two issues of protocol:

• Backwards compatibility isn't strictly an issue here, because _meta isn't stable API. There are certain aspects of _meta that I'd be very pained to change (like the basic field management stuff that has been that way since the beginning), but I don't think parents falls into that list.

• Please don't assign bugs to other people without their permission.

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.