[Django] #24470: Serialization of base classes is not customizable for migrations

10 views
Skip to first unread message

Django

unread,
Mar 10, 2015, 6:31:55 PM3/10/15
to django-...@googlegroups.com
#24470: Serialization of base classes is not customizable for migrations
-------------------------------+--------------------
Reporter: rockymeza | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
I have a base class that is created by a class factory for one of my
models. You can see an example [https://github.com/fusionbox/django-
widgy/blob/master/widgy/models/mixins.py#L97-L117 here], but basically it
works like this:

{{{#!python
def CreateBaseClass(arg):
class cls(object):
def get_arg(self):
return arg

return cls


class MyModel(CreateBaseClass('foo'), models.Model):
# ...
}}}

When I run makemigrations, it serializes the base classes to something
like this:

{{{#!python
migrations.CreateModel(
name='MyModel',
fields=[
('id', models.AutoField(verbose_name='ID',
serialize=False, auto_created=True, primary_key=True)),
],
options={},
bases=(myapp.models.cls, models.Model),
),
}}}

But this errors because myapp.models.cls doesn't exist.

I think that Django should allow the base classes to customize their
serialization so that the migration would end up something like this:

{{{#!python
bases=(myapp.models.CreateBaseClass('foo'), models.Model),
}}}

This could be done through a classmethod called deconstruct_class, and it
would be used like this:

{{{#!python
def CreateBaseClass(arg):
class cls(object):
@classmethod
def deconstruct_class(cls):
return 'myapp.models.CreateBaseClass', (arg,), {}

# ...

return cls
}}}

Here are some related tickets:
#23950, #22951

Thanks,

--
Ticket URL: <https://code.djangoproject.com/ticket/24470>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Mar 10, 2015, 8:27:21 PM3/10/15
to django-...@googlegroups.com
#24470: Serialization of base classes is not customizable for migrations
-------------------------------+--------------------------------------

Reporter: rockymeza | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by charettes):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0


Comment:

Can't you simply define your factored mixin at the module level?

{{{#!python
from django.db import models


def model_mixin_factory(name):
class cls(object):
pass
cls.__name__ = name
return cls

MyMixin = model_mixin_factory('MyMixin')


class MyModel(MyMixin, models.Model):
pass
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/24470#comment:1>

Django

unread,
Mar 11, 2015, 1:12:39 PM3/11/15
to django-...@googlegroups.com
#24470: Serialization of base classes is not customizable for migrations
-------------------------------+--------------------------------------

Reporter: rockymeza | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by gavinwahl):

There are several problems with doing that.

- The `__module__` of MyMixin will be the `__module__` of
model_mixin_factory, not the module were MyMixin was created.
- model_mixin_factory needs to remain backwards compatible. Changing the
signature would break existing code, and there is existing code that uses
it without assigning it to a module-level variable
- It's ugly to specify the name twice

--
Ticket URL: <https://code.djangoproject.com/ticket/24470#comment:2>

Django

unread,
Mar 19, 2015, 1:43:14 PM3/19/15
to django-...@googlegroups.com
#24470: Serialization of base classes is not customizable for migrations
-----------------------------+------------------------------------
Reporter: rockymeza | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-----------------------------+------------------------------------
Changes (by carljm):

* type: Uncategorized => New feature
* stage: Unreviewed => Accepted


Comment:

On the one hand, I think there is definitely a point where we just have to
say "auto-generated migrations are on a best-effort basis, if you're doing
complex things then you will probably have to edit the auto-generated
migrations by hand."

On the other hand, this specific feature request looks relatively elegant
and generic, and I think would probably find use cases beyond this
specific situation , so provided the implementation isn't too complex in
practice I think it's reasonable.

I've marked accepted, but if anyone with more migrations experience thinks
there are serious problems here, feel free to reverse that!

--
Ticket URL: <https://code.djangoproject.com/ticket/24470#comment:3>

Django

unread,
Mar 23, 2015, 10:41:31 AM3/23/15
to django-...@googlegroups.com
#24470: Serialization of base classes is not customizable for migrations
-----------------------------+------------------------------------
Reporter: rockymeza | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: 1.7

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-----------------------------+------------------------------------

Comment (by MarkusH):

It would be nice if we can stick to `deconstruct()` and not introduce
`deconstruct_class()`.

For serializing model managers we follow the pattern `class
MyManager(MyBaseManager.from_queryset(CustomQuerySet)):`
https://docs.djangoproject.com/en/1.8/topics/migrations/#model-managers
which will also work here I think.

I have to think about it a bit more, though, but the general feature seems
useful for more complex projects.

--
Ticket URL: <https://code.djangoproject.com/ticket/24470#comment:4>

Reply all
Reply to author
Forward
0 new messages