Re: Copy-from-base inheritance

157 views
Skip to first unread message

Aymeric Augustin

unread,
Mar 2, 2016, 2:19:23 AM3/2/16
to django-d...@googlegroups.com
Hello,

The “locally declared field gets overriden by parent definition” behavior shown in your example looks counter-intuitive to me.

Inheritance means that children inherit and possibly specialize their parent’s behavior, not that the parent overrides the child.

Best regards,

-- 
Aymeric.

On 02 Mar 2016, at 01:57, Joakim Saario <saario...@gmail.com> wrote:

Hello!

I'd like to propose another inheritance strategy for django's models.

Think of it sort of like reversed abstract models

For example:

class NormalModel(models.Model):
    foo
= models.CharField(max_length=10)
    bar
= models.CharField(max_length=10)

class CopiedBaseModel(NormalModel):
    bar
= models.CharField(max_length=2)
    buzz
= models.CharField(max_length=10)

   
class Meta:
        copy_from_base
= True

Would be equivalent to:

class NormalModel(models.Model):
    foo
= models.CharField(max_length=10)
    bar
= models.CharField(max_length=10)

class CopiedBaseModel(NormalModel):
    foo
= models.CharField(max_length=10)
    bar
= models.CharField(max_length=10)
    buzz
= models.CharField(max_length=10)

My initial use case for this was with django-cms which didn't play well with
multi-table inheritance when i needed to extend a built in plugin of theirs. So
I ended copying the whole model instead, which didn't make me too happy.
So I started writing some code for the behaviour I was after. Which was,
ironicly a standard python inheritance. Django only offers part of this behaviour
through the proxy and abstract models. But neither of them worked for me.

This is quite easy to implement with some of the current abstract model
logic (see the patch).

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/d4872520-eeb1-49a7-a8f3-aefc23a98346%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
<django-copy-from-base.diff>

Joakim Saario

unread,
Mar 2, 2016, 4:25:44 AM3/2/16
to Django developers (Contributions to Django itself)
I'm sorry, this is a typo, local fields overrides parent fields of course. Look at the patch.

Aymeric Augustin

unread,
Mar 2, 2016, 4:52:07 AM3/2/16
to django-d...@googlegroups.com
In that case, I believe this is the ticket you’re looking for: https://code.djangoproject.com/ticket/24305

There seems to be some activity on the related PR: https://github.com/django/django/pull/5122

You may want to review these discussions and see how you can help. Thanks!

-- 
Aymeric.

Joakim Saario

unread,
Mar 2, 2016, 5:02:56 AM3/2/16
to Django developers (Contributions to Django itself)
Yes it is true the patch do include some code for overriding parent fields, but that is not the main feature.

In any way this thread is a double post (I thought the first one go
t lost). Look at: https://groups.google.com/forum/?hl=sv#!topic/django-developers/koRZDDCQREc instead.

Aymeric Augustin

unread,
Mar 2, 2016, 5:59:33 AM3/2/16
to django-d...@googlegroups.com
Now I understand — you want to inherit from a concrete model but to use a separate table.

I’m not sure how widely applicable this would be. I find it counter-intuitive that Parent.objects.all() isn’t a superset of Child.objects.all() anymore. This is a surprising way to implement inheritance.

The usual ways to implement inheritance in an ORM are single-table inheritance (STI) and multi-table inheritance (MTI). Django supports MTI. It also supports inheriting Python classes without affecting the database, with abstract or proxy models, depending on whether the database table(s) are tied to the children or the parent.



In your use case, the only reason why you can’t make the parent model abstract is that it’s provided by a third-party app. Perhaps the general problem is to customize models provided by third-party apps? At this time, Django doesn’t provide a mechanism for doing that. The two solutions I know of are:

1) Swappable models: this is only officially supported for the auth.User model via the AUTH_USER_MODEL setting but it’s technically possible to achieve a similar effect with other models.
2) App-registry hacks: for example, that’s what django-oscar does; I’m not sure they would have ended up with the solution if they had started after Django 1.7, though.

So, in the current state of things, the path of least resistance is to encourage authors of reusable apps to provide an abstract version of inheritable models in addition to the concrete version, like Django does with AbstractUser and User, and perhaps discuss making swappable models a supported API.


If we leave aside the current implementation of abstract models for a moment, for your use case, I can see the logic in having the child model declare what kind of inheritance it uses rather than the parent. In the end, it’s the child’s database table that has either a FK to the parent of a copy of all the parent’s columns (with your patch). `copy_from_base` is a poor name for declaring the whether you’re using MTI or not, though.

If we ever implement single table inheritance (STI), either having the parent declaring that it collects database columns from all its children or the children declare that they add database columns to their parent will be awkward. When inheriting across applications, that will interact very poorly with the migrations framework. So it’s unclear that one solution is better than the other here.

That said, we need to keep `abstract = True` on the parent to prevent the creation of a related database table, which could be accidentally used even if the model was intended to be abstract.

So I don’t think we can draw a general conclusion on whether inheritance behavior should be declared on parents or children.


To sum up, Django currently provides limited but safe options for model inheritance:

- MTI — it's the default when inheriting from a regular model; the main usability drawback is that there’s no good way to specialize a queryset of parents into children
- abstract models (declared on the parent) — the parent cannot interact with the database, each child has its own table, which avoids interactions between the ORM and inheritance
- proxy models (declared on the child) — the child only specializes Python behavior, only the parent has a table, which also avoids interactions between the ORM and inheritance

There has been some talk about introducing STI but I don’t remember seeing a concrete proposal. (Also STI looks suspiciously similar to GFK which aren’t very popular among the core team.)

Looking at where your proposal would fit in this landscape, it’s unclear to me that the benefits outweigh the bizarre result of making both Parent.objects.all() and Child.objects.all() work, but not have the latter be a subset of the former.

I would be more comfortable with providing guidelines for reusable apps whose models may be inherited.


I suppose I just spent two pages rebuilding the reasoning behind the design of abstract models… I hope this helps anyway!

-- 
Aymeric.


Joakim Saario

unread,
Mar 3, 2016, 5:03:55 AM3/3/16
to Django developers (Contributions to Django itself)
I'm aware that the name of the Meta-option isn't the best (it was just a working name for me)
or that it should be implemented like a Meta-option at all.

While this may be counter-intuitive from the ORM-perspective, it isn't counter-intuitive at all
from the regular python-programmer's perspective, but rather the other way around. This is
why I think it would be great to at least have some kind of way to to regular class-inheritance,
in addition to relational inheritance.

The problem is that django hijacks the class-inheritance feature of python and uses it soley
for relational inheritance.

I'm not a fan of doing hacks either, of course you can do monkey-patching to
achive this without changes in django core, it's python after all.

Aymeric Augustin

unread,
Mar 3, 2016, 6:36:08 AM3/3/16
to django-d...@googlegroups.com
Hi Joakim,

On 03 Mar 2016, at 11:03, Joakim Saario <saario...@gmail.com> wrote:

> The problem is that django hijacks the class-inheritance feature of python and uses it soley
> for relational inheritance.


I understand that you would like each model class to have its own table with all its fields,
whether they’re defined in this class or in a superclass.

However, according to the regular OOP model, Django considers that children ORM classes
specialize their parent class. You must be able to take a child instance and cast it to a parent
instance by throwing away attributes not used in the parent and using the parent’s methods.

In other words, if Child.objects.get(pk=1) works, Parent.objects.get(pk=1) is supposed to give
you a Parent instance that is a more generic version of the Child instance. (I have to admit
that this isn’t always true with Django. If the Parent is abstract, Parent.objects.get will raise
an exception.)

My main concern with your proposal is that Parent.objects.get(pk=1) would return an object
that is totally unrelated to Child.objects.get(pk=1)! I have a hard time believing this qualifies
as “regular class inheritance”. I suspect many Python programmes would find it surprising.

That is why Django cannot ignore the fact that an ORM model is backend by a database
table. It needs to decide what table to use — or disable database access entirely to avoid
this problem.

I’m sorry if this feels like “hijacking” to you. It’s prevents data corruption bugs, though.

Best regards,

--
Aymeric.

Joakim Saario

unread,
Mar 3, 2016, 12:58:31 PM3/3/16
to Django developers (Contributions to Django itself)
Yes, Child would be "unrelated" in the sense that it doesn't have any connections to its parent
other than the fields. Is the problem that this uses the inheritance semantics?

Python doesn't offer a mechanism for accessing parent objects or vice versa, this is something you
will have to implement yourself if you want it. So I don't see how this would be surprising for
any python programmers.

The relational inheritance can actually cause data corruption when you don't expect the parents
data to be a superset of the childrens data. However, the the parent's definition may still be
a superset of the childrens definition, if you don't override any fields.

As an example of this, consider:

class 3DTable(models.Model):
    width
= models.IntegerField()
    height
= models.IntegerField()
    depth
= models.IntegerField()

class 4DTable(3DTable):
    some_other_dimension
= models.IntegerField()    

Now, of course this depend of what a 4DTable is. It may NOT even be projectible in the 3D space at all, at least not in a sensible way. So it s houldn't be presented as if it were.

As for the definition, they are mostly the same, 4DTable adds another dimension. It should in this case inherit the definition only.

Shai Berger

unread,
Mar 3, 2016, 3:08:18 PM3/3/16
to django-d...@googlegroups.com
Hi Joakim,

You seem to be taking the very view that inheritance means nothing but reuse
of definitions. This view, however, is not accepted in any principled approach
to OOP that I'm aware of.

On Thursday 03 March 2016 19:58:30 Joakim Saario wrote:
> Yes, Child would be "unrelated" in the sense that it doesn't have any
> connections to its parent
> other than the fields. Is the problem that this uses the inheritance
> semantics?
>

Mostly, yes; inheritance encodes an "is-a" relation between concepts, which
you seem to disregard completely.

> Python doesn't offer a mechanism for accessing parent objects or vice
> versa, this is something you will have to implement yourself if you want it.
> So I don't see how this would be surprising for any python programmers.
>

There is a difference in terms here: Aymeric referred to "casting into
parents", you are referring to "accessing parent objects". The point is that
the parent object is not some distinct entity that you need to navigate to,
but just a different way to look at the same instance.

Python does support a form of this casting to a parent with super().

> The relational inheritance can actually cause data corruption when you
> don't expect the parents data to be a superset of the childrens data.

If you don't expect this, your understanding of inheritance is at odds with
the rest of the world's.

> However, the the parent's
> definition may still be
> a superset of the childrens definition, if you don't override any fields.
>
> As an example of this, consider:
>
> class 3DTable(models.Model):
> width = models.IntegerField()
> height = models.IntegerField()
> depth = models.IntegerField()
>
> class 4DTable(3DTable):
> some_other_dimension = models.IntegerField()
>
> Now, of course this depend of what a 4DTable is. It may NOT even be
> projectible in the 3D space at all, at least not in a sensible way. So it s
> houldn't be presented as if it were.
>

If so, it shouldn't be a subclass of 3DTable.

> As for the definition, they are mostly the same, 4DTable adds another
> dimension. It should in this case inherit the definition only.
>

If the two classes share nothing but a set of fields -- and these fields do not
make sense as some independent concept that is common to the two classes --
then the best design is to keep them completely separate (this is true even if
they are not Django models), because they are likely to evolve independently.

If the shared part does make sense as a full concept, but you still intend the
two classes to be unrelated, then that concept should be defined as a common
parent class (or abstract model):

class BaseDimensions(models.Model):
width = models.IntegerField()
height = models.IntegerField()
depth = models.IntegerField()
class Meta:
abstract = True

class 3DTable(BaseDimensions):
pass

class 4DTable(BaseDimensions):
some_other_dimension = models.IntegerField()


HTH,
Shai.
Reply all
Reply to author
Forward
0 new messages