#27914 closed New feature (fixed)
makemigrations produces incorrect path for inner classes
Reported by: | Serge van den Boom | Owned by: | Hasan Ramezani |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | anand21nanda@…, Hasan Ramezani | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
When you define a subclass from django.db.models.Field
as an inner class of some other class, and use this field inside a django.db.models.Model
class, then when you run manage.py makemigrations
, a migrations file is created which refers to the inner class as if it were a top-level class of the module it is in.
To reproduce, create the following as your model:
class Outer(object): class Inner(models.CharField): pass class A(models.Model): field = Outer.Inner(max_length=20)
After running manage.py makemigrations
, the generated migrations file contains the following:
migrations.CreateModel( name='A', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('field', test1.models.Inner(max_length=20)), ], ),
Note the test1.models.Inner
, which should have been test1.models.Outer.Inner
.
The real life case involved an EnumField from django-enumfields, defined as an inner class of a Django Model class, similar to this:
import enum from enumfields import Enum, EnumField class Thing(models.Model): @enum.unique class State(Enum): on = 'on' off = 'off' state = EnumField(enum=State)
This results in the following migrations code:
migrations.CreateModel( name='Thing', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('state', enumfields.fields.EnumField(enum=test1.models.State, max_length=10)), ], ),
This refers to test1.models.State
, instead of to test1.models.Thing.State
.
Attachments (2)
Change History (26)
comment:1 by , 8 years ago
Type: | Uncategorized → Bug |
---|
comment:2 by , 8 years ago
Component: | Database layer (models, ORM) → Migrations |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → New feature |
Version: | 1.10 → master |
comment:3 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 8 years ago
Patch needs improvement: | set |
---|---|
Summary: | makemigrations produces incorrect path for subclasses of Field which are inner classes → makemigrations produces incorrect path for inner classes |
I think we should focus on using __qualname__
during migration serialization as well instead of simply solving the field subclasses case.
comment:8 by , 5 years ago
I am still encountering this issue when running makemigrations
on models that include a django-enumfields EnumField. From tracing through the code, I believe the Enum is getting serialized using the django.db.migrations.serializer.TypeSerializer, which still uses the __name__
rather than __qualname__
. As a result, the Enum's path gets resolved to app_name.models.enum_name
and the generated migration file throws an error "app_name.models has no 'enum_name' member". The correct path for the inner class should be app_name.models.model_name.enum_name
.
https://github.com/django/django/blob/master/django/db/migrations/serializer.py#L266
comment:9 by , 5 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
Reopening it. Will recheck with nested enum field.
comment:10 by , 5 years ago
Cc: | added |
---|
comment:11 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:15 by , 5 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:18 by , 5 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
commit 6452112640081ac8838147a8ba192c45879203d8 does not resolve this ticket.
The commit patched the EnumSerializer
with __qualname__
, which works for Enum members.
However, the serializer_factory
is returning TypeSerializer
for the Enum subclass, which is still using __name__
With v3.0.x introducing models.Choices, models.IntegerChoices, using nested enums will become a common pattern; serializing them properly
with __qualname__
seems prudent.
Here's a patch for the 3.0rc1 build
https://github.com/django/django/files/3879265/django_db_migrations_serializer_TypeSerializer.patch.txt
comment:19 by , 5 years ago
Has patch: | unset |
---|---|
Patch needs improvement: | unset |
Agreed, we should fix this.
by , 5 years ago
Attachment: | 0001-Fixes-27914-Fixed-serialization-of-nested-classes-se.patch added |
---|
comment:22 by , 5 years ago
Has patch: | set |
---|
by , 5 years ago
Attachment: | 0002-Fixed-WriterTests.test_deconstruct_class_arguments-t.patch added |
---|
This should be possible to do by relying on
__qualname__
(instead of__name__
) now that master is Python 3 only.