Support dictionaries in Field.choices for named groups.

[Tom Forbes]

The Django documentation gives this example for creating named groups of choices:

MEDIA_CHOICES = [
    ('Audio', (
            ('vinyl', 'Vinyl'),
            ('cd', 'CD'),
        )
    ),
    ('Video', (
            ('vhs', 'VHS Tape'),
            ('dvd', 'DVD'),
        )
    ),
    ('unknown', 'Unknown'),
]

With Python 3.7 (and implicitly in CPython 3.6) dictionaries are ordered, meaning this syntax could be replaced by the cleaner and easier to parse:

{
    "Audio": (
         ('vinyl', 'Vinyl'),
         ('cd', 'CD'),
    ),
    "Video": (
        ('vhs', 'VHS Tape'),
        ('dvd', 'DVD'),
    ),
   "unknown": "Unknown"
}

Once 3.7 is the lowest supported version we should document that this is supported, and ensure that it works correctly.

[Mariusz Felisiak]

Agreed, we can officially add this even in Django 3.1, because we've already made a decision to assume that dictionaries preserve ordering (see #30159 and ​a short discussion).

[Tom Forbes]

Do we want to support this as an additional way to define choices or make it the preferred way? Even for non-grouped choices I think it looks better:

    YEAR_IN_SCHOOL_CHOICES = {
        'FR': 'Freshman',
        'SO': 'Sophomore',
        'JR': 'Junior',
        'SR': 'Senior',
        'GR': 'Graduate',
    }

I can either replace the current examples with the dictionary syntax and include a note that you can also pass a list of tuples as well, or do it the other way round. Or just ignore standard choices for now and focus on grouped choices only?

[Adam Johnson]

+1 for preferred

[Tom Forbes]

PR: ​https://github.com/django/django/pull/12449

This was actually hard to test. We have a lot of tests that use the list syntax and it's really not easy to parametrize them to test with the different ways of defining choices. I've added a few tests for the specific functionality I've added - three supported variants:

1. the current format, lists of tuples
2. dictionaries containing tuples: {"name": [(1, "one")]}
3. nested dictionaries: {"name": {1: "one"}}

Under the hood the dictionary syntax is flattened into a list of tuples for compatibility.

[Carlton Gibson]

Hi Tom: as per comments on the PR, my main (only really) concern is the phrasing on the checks and docs: it's minor but I think if we can be super precise/clear we'll save a lot of confusion.

Other than that I quite like it: using a dictionary does look much cleaner.
Let's see if we can pin down the wording.
Thanks!

[Tom Forbes]

Thanks for the review! I've attempted to address your comments, but you're right - this is a hard thing to find the correct wording for.

[Carlton Gibson]

Given Tom's last comment ref the docs/wording, I'd forgotten why this has sat here so long.

It's because there's a pending question on the PR, with regards to a performance regression, depending on the widget in play, when passing choices explicitly to a field.

• Discussion begins here: ​https://github.com/django/django/pull/12449#pullrequestreview-527073608
• Reproduce is here: ​https://github.com/django/django/pull/12449#pullrequestreview-527073608

I don't think we can just ignore the performance regression. Nick's last comment suggests a way forward:

This is why I believe that we should fix the inheritance so that RelatedFieldWidgetWrapper subclasses ChoicesWidget.

But also I think we need at least some internal docstrings/comments, and maybe some public documentation. Tom came back to the issue with "… I've lost most of the context 🙈". I was going through it again this morning "Oh, yes all looks good — what's the hold-up?", then "Oh, yes, I remember this…" — the new use of normalize_field_choices() — and the requirement to go via the ChoiceField.choices setter is (fine, perhaps, but) a little particular… — Coming back to it, it takes a while to load into RAM. There will be custom fields out there will need to adapt. In both case some extra guidance would be good.

Overall I think the change is good. We should go with it. (Unless you have energy to pick it up now Tom, I'll add it to my long list for the next cycle.)

[Nick Pope]

Updated ​PR

[GitHub <noreply@…>]

In 500e0107:

Fixed #31262 -- Added support for mappings on model fields and ChoiceField's choices.

[GitHub <noreply@…>]

In 8c8cbe66:

Refs #31262 -- Renamed ChoiceIterator to BaseChoiceIterator.

Some third-party applications, e.g. django-filter, already define
their own ChoiceIterator, so renaming this BaseChoiceIterator will
be a better fit and avoid any potential confusion.

See ​https://github.com/carltongibson/django-filter/pull/1607.