[Django] #24340: Nested deconstruction does not descend into lists, tuples or dicts
Django
----------------------------+--------------------
Reporter: gasman | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
`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!)
--
Ticket URL: <https://code.djangoproject.com/ticket/24340>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: gasman | Owner: gasman
Type: Bug | Status: assigned
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 gasman):
* owner: nobody => gasman
* needs_docs: => 0
* status: new => assigned
* needs_tests: => 0
* needs_better_patch: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24340#comment:1>
Django
Reporter: gasman | Owner: gasman
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by gasman):
* has_patch: 0 => 1
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24340#comment:2>
Django
Reporter: gasman | Owner: gasman
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24340#comment:3>
Django
Reporter: gasman | Owner: gasman
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by gasman):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24340#comment:4>
Django
Reporter: gasman | Owner: gasman
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Changes (by gasman):
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24340#comment:5>
Django
Reporter: gasman | Owner: gasman
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by gasman):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24340#comment:6>
Django
Reporter: gasman | Owner: gasman
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by gasman):
@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).
--
Ticket URL: <https://code.djangoproject.com/ticket/24340#comment:7>
Django
Reporter: gasman | Owner: gasman
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* needs_docs: 1 => 0
* version: 1.7 => master
Comment:
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 :)
--
Ticket URL: <https://code.djangoproject.com/ticket/24340#comment:8>
Django
Reporter: gasman | Owner: gasman
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by gasman):
Thanks! Rebased / squashed now.
--
Ticket URL: <https://code.djangoproject.com/ticket/24340#comment:9>
Django
Reporter: gasman | Owner: gasman
Component: Migrations | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markus Holtermann <info@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"ff8a02ae0b33ee52050e22909d432a3aa060d683" ff8a02a]:
{{{
#!CommitTicketReference repository=""
revision="ff8a02ae0b33ee52050e22909d432a3aa060d683"
Fixed #24340 -- Added nested deconstruction for list, tuple and dict
values
Nested deconstruction should recursively deconstruct items within list,
tuple and dict values.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24340#comment:10>