[Django] #24470: Serialization of base classes is not customizable for migrations
Django
-------------------------------+--------------------
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
Reporter: rockymeza | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.7
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
Reporter: rockymeza | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 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
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
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
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
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>