Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#30505 closed Cleanup/optimization (fixed)

Ordering of Field.choices is significant for makemigrations

Reported by: Marnanel Thurman Owned by: Caio Ariede
Component: Migrations Version: dev
Severity: Normal Keywords:
Cc: Carlton Gibson Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

makemigrations regards the ordering of Field.choices as significant. The docs require Field.choices to be an iterable, but not to be well-ordered. If Field.choices is not well-ordered, then almost every run of makemigrations will treat the model as changed, even without changes to the code.

The ordering of Field.choices is not significant anywhere else, except as a cosmetic detail in the admin.

This bug even occurs in

ACTIVITY_TYPES = set(['Create', 'Update', 'Delete'])

[....]
    f_type = models.CharField(
            max_length=255,
            choices=[(x,x) for x in ACTIVITY_TYPES],
            )

I would have expected the value of Field.choices here to be well-ordered (and sorted) but makemigrations still flags this as changed every time.

I can put a test case together if you like.

Attachments (1)

ticket30505.tar.bz2 (3.9 KB ) - added by Marnanel Thurman 6 years ago.
Test case. Running makemigrations will create new migrations indefinitely

Download all attachments as: .zip

Change History (9)

comment:1 by Carlton Gibson, 6 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

I can put a test case together if you like.

Yes, please. Or a sample project that we can just run to play with. (Pending that I'll assume it reproduces. 🙂)

I need to look deeper to see exactly what's going on here, but there's at least a documentation issue:

An iterable (e.g., a list or tuple) ...

If it's required to be ordered it should say sequence or similar.

I'd suspect it's just been assumed that a list or tuple would be used. Maybe we can relax that. If not we'll have to change the docs.

comment:2 by Carlton Gibson, 6 years ago

Cc: Carlton Gibson added

by Marnanel Thurman, 6 years ago

Attachment: ticket30505.tar.bz2 added

Test case. Running makemigrations will create new migrations indefinitely

comment:3 by Caio Ariede, 6 years ago

Owner: changed from nobody to Caio Ariede
Status: newassigned

comment:4 by Caio Ariede, 6 years ago

Has patch: set

comment:5 by Mariusz Felisiak, 6 years ago

Has patch: unset
Type: BugCleanup/optimization

This should be clarified in documentation, maybe "A :term:`sequence`" instead of "An iterable ..." or a note about ordering.

comment:6 by Caio Ariede, 6 years ago

Has patch: set

new PR

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In 5248abe9:

Fixed #30505 -- Doc'd how changes in the order of Field.choices affect migrations.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

In d6d65c1:

[2.2.x] Fixed #30505 -- Doc'd how changes in the order of Field.choices affect migrations.

Backport of 5248abe9b0425c1fc989c60a55860cdb4d135bcf from master

Note: See TracTickets for help on using tickets.
Back to Top