Proposed opt_group argument for ModelChoiceField and ModelMultipleChoiceField

[Héctor Urbina]

Hello,

I've just implemented this and I thought It could well be incorporated into Django itself; I guess it's a fairly common feature that one may need on any project.

What I propose is to add an optional opt_group argument to ModelChoiceField and ModelMultipleChoiceField; which indicates the item's field whose value is used to group the choices. It should be used in conjunction with a queryset which is (primarily) sorted by the same field.

Let me show with an example:

class Category(models.Model):
    name = models.CharField(max_length=20)

class Item(models.Model):
    name = models.CharField(max_length=20)
    category = models.ForeignKey(Category)

And in some form's initialization process

field = ModelChoiceField(queryset=Item.objects.order_by('category__name', 'name'), opt_group='category')

field.choices will dynamically collect choices into named groups as a 2-tuple, which the underlying widget should present using optgroup HTML elements.

[Tim Graham]

At first glance, that looks more appropriate as a widget option since ModelChoiceField isn't required to use a Select widget.

[Héctor Urbina]

Replying to Tim Graham:

At first glance, that looks more appropriate as a widget option since ModelChoiceField isn't required to use a Select widget.

The default ModelChoiceField's widget (Select) is prepared to understand 2-tuples coming on the field's choices attribute and translates them to optgroups.
ModelChoiceField uses ModelChoiceIterator to generate the choices; that is the actual class that is doing the job on ​my current implementation. My idea is to include support for optgroup, not require it; I'm updating my proposal to clarify that.

[Tim Graham]

I'm still unconvinced that adding an argument with an HTML looking name to a model field that only affects selected widgets is a good design. I see that it's convenient compared to the alternative of providing a custom ModelChoiceIterator but I don't know that the coupling is a good design.

[Tim Graham]

As the discussion has stalled here, you can write to the DevelopersMailingList for other opinions. Thanks.

[Simon Charette]

By the way, this is currently possible with the following code

from functools import partial
from itertools import groupby
from operator import attrgetter

class GroupedModelChoiceIterator(ModelChoiceIterator):
    def __init__(self, field, groupby):
        self.groupby = groupby
        super().__init__(field)

    def __iter__(self):
        if self.field.empty_label is not None:
            yield ("", self.field.empty_label)
        queryset = self.queryset
        # Can't use iterator() when queryset uses prefetch_related()
        if not queryset._prefetch_related_lookups:
            queryset = queryset.iterator()
        for group, objs in groupby(queryset, self.groupby):
            yield (group, [self.choice(obj) for obj in objs])

class GroupedModelChoiceField(ModelChoiceField):
    def __init__(self, *args, choices_groupby, **kwargs):
        if isinstance(choices_groupby, str):
            choices_groupby = attrgetter(choices_groupby)
        self.iterator = partial(GroupedModelChoiceIterator, groupby=choices_groupby)
        super().__init__(*args, **kwargs)

While I won't push to get this feature included I think that a request for feedback on django-developers has a good chance of being accepted. It looks like the main argument against closing this ticket as _wontfix_ was the naming of the ModelChoice(opt_group) argument which was effectively leaking HTML implementation details at the form layer.

[angelafoli]

Replying to Simon Charette:

By the way, this is currently possible with the following code

from functools import partial
from itertools import groupby
from operator import attrgetter

class GroupedModelChoiceIterator(ModelChoiceIterator):
    def __init__(self, field, groupby):
        self.groupby = groupby
        super().__init__(field)

    def __iter__(self):
        if self.field.empty_label is not None:
            yield ("", self.field.empty_label)
        queryset = self.queryset
        # Can't use iterator() when queryset uses prefetch_related()
        if not queryset._prefetch_related_lookups:
            queryset = queryset.iterator()
        for group, objs in groupby(queryset, self.groupby):
            yield (group, [self.choice(obj) for obj in objs])

class GroupedModelChoiceField(ModelChoiceField):
    def __init__(self, *args, choices_groupby, **kwargs):
        if isinstance(choices_groupby, str):
            choices_groupby = attrgetter(choices_groupby)
        elif not callable(choices_groupby):
            raise TypeError('choices_groupby must either be a str or a callable accepting a single argument')
        self.iterator = partial(GroupedModelChoiceIterator, groupby=choices_groupby)
        super().__init__(*args, **kwargs)

While I won't push to get this feature included I think that a request for feedback on django-developers has a good chance of being accepted. It looks like the main argument against closing this ticket as _wontfix_ was the naming of the ModelChoice(opt_group) argument which was effectively leaking HTML implementation details at the form layer.

This solution no longer works as of this change: ​https://github.com/django/django/commit/5ec64f96b2d83ec3c0ef574f52e4767a440017b8