Order of "choices" on CharField randomly changing forcing new migrations despite no changes.

[Karl]

I have a field attachment_type in a class that uses

TYPE_CHOICES = {
        ("image", "Image"),
        ("document", "Document"),
        ("audio", "Audio"),
        ("video", "Video"),
    }

Every makemigrations call generates a new migration for that field:

migrations.AlterField(
            model_name="genericattachment",
            name="attachment_type",
            field=models.CharField(
                choices=[
                    ("audio", "Audio"),
                    ("document", "Document"),
                    ("video", "Video"),
                    ("image", "Image"),
                ],
                default="image",
                max_length=10,
            ),
        ),

I noticed that the order of the choices in the migration changes every time. It doesn't seem to have a consistent method to compare the set of tuples properly and so it generates a new migration every time. I have not yet observed that it even accidentally collides in a way that does not generate a new one.

[Natalia Bidart]

Hello WoosterInitiative, thank you for taking the time to create this report.

This ticket is, strictly speaking, a duplicate of #26609, which was closed as wontfix for the reasons detailed in the ticket:26609#comment:3. But I'm happy to say that you can also workaround this issue by defining a callable for your choices (see #24561 and ​release notes), and this will avoid having migrations being generated because the order of the choices changed.

[Claude Paroz]

I don't think that a set() is a supported data structure for model field choices. Maybe we should replace A mapping or iterable in the format described below by A mapping or sequence in the format described below in ​the docs?

[Natalia Bidart]

Replying to Claude Paroz:

I don't think that a set() is a supported data structure for model field choices. Maybe we should replace A mapping or iterable in the format described below by A mapping or sequence in the format described below in ​the docs?

Code wise, we really do not mind that these are sets... Do you see a concrete issue with using sets? (other than dropdowns or options being shown in a random order).

[Karl]

Replying to Claude Paroz:

I don't think that a set() is a supported data structure for model field choices. Maybe we should replace A mapping or iterable in the format described below by A mapping or sequence in the format described below in ​the docs?

Figured this out. It isn't explicitly *not* allowed, but it causes the issue I reported. If it is going to be allowed in the docs explicitly, migrations should handle it well.

Beyond that, I don't know why one would need a set(). Pretty sure I was just combining a couple of sections of the docs when I wrote it because I can't ever remember the specific syntax and made a mistake.

Curious if a check could be created to prevent my (and other's) future confusion?

[Claude Paroz]

Replying to Natalia Bidart:

Do you see a concrete issue with using sets? (other than dropdowns or options being shown in a random order).

The main issue is that in 99% of the case, it will be an oversight by the dev, and cause the issue at hand. Do you see a valid use case using sets? I do agree with the reporter that if we support sets(), we should also fix the migration issue.

[Natalia Bidart]

Replying to Claude Paroz:

Replying to Natalia Bidart:

Do you see a concrete issue with using sets? (other than dropdowns or options being shown in a random order).

The main issue is that in 99% of the case, it will be an oversight by the dev, and cause the issue at hand. Do you see a valid use case using sets?

Thank you for the response! My use case is, perhaps, a little simplistic or even a stretch, but let me share it:

You want your site users to pay attention when choosing an option so you use a set to have a non deterministic order of the displayed options.

I do agree with the reporter that if we support sets(), we should also fix the migration issue.

I disagree with this one. If a Django user uses a set(), I think they may be up to two goals:

1. Either they made a mistake, and the infinite migrations make them notice (and fix that), or
2. They intentionally did this and solve the migration issue by using a callable.

In any case, unordered choices and the migration issues is really what's #26609 is about (back when dicts weren't sorted), which was wontfixed. Perhaps disallowing unordered iterables should be discussed and potentially re-opened there? Thanks!

[Claude Paroz]

Then I guess we are at a point where we agree to disagree :-)

[Karl]

Replying to Natalia Bidart:

I disagree with this one. If a Django user uses a set(), I think they may be up to two goals:

1. Either they made a mistake, and the infinite migrations make them notice (and fix that), or
2. They intentionally did this and solve the migration issue by using a callable.

Here's where I think that's a mistake: I didn't notice it right away and created several migrations (3?) before noticing. I had even patched my prod environment with it. A simple "check" on the model (not explicitly part of migrations) likely would have caught it. Instead, I broke my migrations trying to "undo" it and lost some data.

Of course, this is in a one-man-shop environment with no review, so perhaps it's not a real issue on the global scale.

A lot of that is on me, but I don't see a "check" as a large burden and it can be ignored if the dev is so inclined.

[Natalia Bidart]

Claude, Karl, your points make sense and I'm happy to go with the majority here. We already have ​a check for choices, it would be trivial to check against "unorderable iterables":

        if not isinstance(self.choices, Iterable) or isinstance(self.choices, str):
            return [
                checks.Error(
                    "'choices' must be a mapping (e.g. a dictionary) or an iterable "
                    "(e.g. a list or tuple).",
                    obj=self,
                    id="fields.E004",
                )
            ]

I'm thinking we may need to reopen #29609 and repurpose that ticket to grow the system check for choices? Claude what do you think?

[Claude Paroz]

Replying to Natalia Bidart:

I'm thinking we may need to reopen #26609 and repurpose that ticket to grow the system check for choices? Claude what do you think?

Sure, +1!