Opened 11 years ago
Closed 11 years ago
#22951 closed Bug (fixed)
Adding 'deconstruct' method breaks serialization of type
| Reported by: | Owned by: | jambonrose | |
|---|---|---|---|
| Component: | Migrations | Version: | 1.7-rc-1 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description
A custom field may take a type as as argument (for example, an Enum type). Normally, class references are one of the things the migrations framework can serialize by default (https://docs.djangoproject.com/en/dev/topics/migrations/#serializing-values). However, we probably also want to serialize the value, otherwise we get an error (see ValueEnum in the example).
To serialize the value, we add a deconstruct method (in the example of ValueEnum2, this uses the deconstructible decorator). I would expect that, after adding this, the field would work. However, django.db.migrations.autodetector.deep_deconstruct tries to call deconstruct on the class.
ValueEnum3 is an example of a workaround, which should probably not be required.
I would expect Django to either check whether something is a type before calling deconstruct, or to not call deconstruct if it is an instance method (e.g. you might want to be able to also add a deconstruct classmethod to customize deconstruction of the class).
# Requires Python 3.4 or the 'enum34' package import enum import types from django.db import models from django.utils.deconstruct import deconstructible from django.utils.six import with_metaclass class SimpleEnumField(with_metaclass(models.SubfieldBase, models.IntegerField)): def __init__(self, enum_type, **kwargs): self.enum = enum_type choices = [(e.value, e.name) for e in enum_type] super(SimpleEnumField, self).__init__(choices=choices, **kwargs) def deconstruct(self): name, path, args, kwargs = super(SimpleEnumField, self).deconstruct() kwargs['enum_type'] = self.enum kwargs.pop('choices', None) return name, path, args, kwargs def get_prep_value(self, value): return value.value def to_python(self, value): return self.enum(value) class ValueEnum(enum.Enum): """ Django will be able to serialize the type, but not the instance:: ValueError: Cannot serialize: <ValueEnum.A: 0> """ A = 0 B = 1 @deconstructible class ValueEnum2(enum.Enum): """ The deconstructible decorator allows Django to serialize the instances, but breaks serialization of the type--it will try to call deconstruct on the type, but it will fail because it is an instance method:: TypeError: deconstruct() missing 1 required positional argument: 'obj' """ A = 0 B = 1 def deconstruct_enum(self): return ( '%s.%s' % (self.__class__.__module__, self.__class__.__name__), (self.value,), {} ) class ValueEnum3(enum.Enum): """ Workaround: if we add the deconstruct method to the instances, but not to the type, then Django will do the right thing. """ A = 0 B = 1 def __init__(self, value): self.deconstruct = types.MethodType(deconstruct_enum, self) class Foo(models.Model): # This field works field1 = SimpleEnumField(ValueEnum) # ValueError: Cannot serialize: <ValueEnum.A: 0> # field2 = SimpleEnumField(ValueEnum, default=ValueEnum.A) # TypeError: deconstruct() missing 1 required positional argument: 'obj' # field3 = SimpleEnumField(ValueEnum2, default=ValueEnum2.A) field4 = SimpleEnumField(ValueEnum3, default=ValueEnum3.A)
Change History (9)
comment:1 by , 11 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 11 years ago
| Easy pickings: | set |
|---|
comment:3 by , 11 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:5 by , 11 years ago
| Has patch: | set |
|---|
comment:6 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
comment:8 by , 11 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → new |
I don't think this was fixed everywhere it needed to be -- the migration writer is still calling deconstruct on types here: https://github.com/django/django/blob/master/django/db/migrations/writer.py#L334
Traceback (most recent call last):
File "./manage.py", line 11, in <module>
execute_from_command_line(sys.argv)
File "django/django/core/management/__init__.py", line 385, in execute_from_command_line
utility.execute()
File "django/core/management/__init__.py", line 377, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "django/core/management/base.py", line 288, in run_from_argv
self.execute(*args, **options.__dict__)
File "django/core/management/base.py", line 338, in execute
output = self.handle(*args, **options)
File "django/core/management/commands/makemigrations.py", line 124, in handle
self.write_migration_files(changes)
File "django/core/management/commands/makemigrations.py", line 152, in write_migration_files
migration_string = writer.as_string()
File "django/db/migrations/writer.py", line 131, in as_string
operation_string, operation_imports = OperationWriter(operation).serialize()
File "django/db/migrations/writer.py", line 88, in serialize
arg_string, arg_imports = MigrationWriter.serialize(arg_value)
File "django/db/migrations/writer.py", line 331, in serialize
return cls.serialize_deconstructed(path, args, kwargs)
File "django/db/migrations/writer.py", line 239, in serialize_deconstructed
arg_string, arg_imports = cls.serialize(arg)
File "django/db/migrations/writer.py", line 334, in serialize
return cls.serialize_deconstructed(*value.deconstruct())
TypeError: unbound method deconstruct() must be called with FooEnum instance as first argument (got nothing instead)
comment:9 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Could you please open a new ticket? The fix for this one was already released. Thanks!
This should be an easy change to deep_deconstruct to change "doesn't have deconstruct: return" to "doesn't have deconstruct or is a type: return".