Opened 10 years ago
Closed 9 years ago
#24340 closed Bug (fixed)
Nested deconstruction does not descend into lists, tuples or dicts
Reported by: | Matt Westcott | Owned by: | Matt Westcott |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
django.db.migrations.autodetector.deep_deconstruct
returns list, tuple and dict values untouched, rather than recursively deconstructing the items within those collections. This causes problems when those items are objects with deconstruct
methods, which compare as equal when deconstructed but not when compared directly - in these cases, the autodetector will wrongly report a change to the field.
(I'll share the details of my rather esoteric use case if you really want, but given that deep_deconstruct
already includes specific handling for classes and fields as parameters of a model field, I dare say that it's not too much of a leap to include these types as well!)
Change History (10)
comment:1 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 10 years ago
Has patch: | set |
---|
comment:3 by , 10 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
comment:4 by , 10 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
comment:5 by , 10 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
comment:6 by , 10 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
comment:7 by , 10 years ago
@MarkusH - regarding the needs_documentation flag, is that something I can usefully work on? If so, can you let me know what sort of documentation you have in mind, as this doesn't seem like a particularly user-facing part of Django...?
The nearest thing in the docs is probably https://docs.djangoproject.com/en/dev/topics/migrations/#serializing-values , and this already states that list/tuple/dict are supported (which is correct - serialization in general works, it's only the autodetector that has problems).
comment:8 by , 10 years ago
Needs documentation: | unset |
---|---|
Version: | 1.7 → master |
Hey gasman. I'm sorry that this ticket somehow got under my radar.
The only thing that was missing when I set the flag were release notes. But I wouldn't backport it now, so it will be in 1.9. Thus the only thing that needs to be done is squashing the commits and rebasing them onto the current master and make sure all tests pass.
Thanks :)
PR: https://github.com/django/django/pull/4129
Note that I've stopped short of supporting deconstruction of sets and dict keys, because the return value of
deep_deconstruct
is not hashable.