[Django] #30920: Choice enumeration types should not enforce enum.unique(). It does not mean what it seems like it should mean.

Django

unread,

Oct 27, 2019, 11:17:29 PM10/27/19

to django-...@googlegroups.com

#30920: Choice enumeration types should not enforce enum.unique(). It does not mean
what it seems like it should mean.
-------------------------------------+-------------------------------------
Reporter: stevecj | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 3.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The stated reason that Choice enumeration types enforce `enum.unique()` is
"to ensure that values cannot be defined multiple times" but that's a
misunderstanding of how `Enum` works with values and what `enum.unique()`
is for.

`Enum` already enforces that no 2 instances may have the same value, but
by default, it allows 2 or more attributes to refer to that same instance.

I believe that is desirable to allow situations like the following:

{{{
class Vehicle(models.TextChoices):
CAR = 'C'
TRUCK = 'T'
JET_SKI = 'J'
DEFAULT = 'C'
}}}

With `enum.unique()` ''not'' enforced, this will create 3 entries (not 4)
and make the single `CAR` entry instance available through either
`Vehicle.CAR` or `Vehicle.DEFAULT`. By enforcing `enum.unique()`, we cause
an exception to be raised at `DEFAULT = 'C'` instead of making an alias to
`Vehicle.CAR`, and that appears to be purely a limitation and not a
benefit.

--
Ticket URL: <https://code.djangoproject.com/ticket/30920>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Oct 28, 2019, 4:07:33 AM10/28/19

to django-...@googlegroups.com

#30920: Choice enumeration types should not enforce enum.unique().

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* status: new => closed
* cc: Shai Berger, Nick Pope (added)
* type: Bug => New feature
* resolution: => wontfix

Comment:

Thanks for this report, however I don't think that it's the desired
behavior. All properties (`.names`, `.labels`, `.choices`, `.names` ) will
return only the first value without enforcing uniqueness, for example:
{{{
class Fruit(models.IntegerChoices):
APPLE = 1, 'Apple'
PINEAPPLE = 1, 'Pineapple'
DEFAULT 1

>>> Fruit.choices
[(1, 'Apple')]
>>> Fruit.labels
['Apple']
>>> Fruit.values
[1]
>>> Fruit.names
['APPLE']
}}}

What can be confusing. We made few design decisions to create the most
usable and less error-prone classes for defining choices, IMO. `Choices`
is not a strict `enum` implementation.

--
Ticket URL: <https://code.djangoproject.com/ticket/30920#comment:1>

Django

unread,

Oct 28, 2019, 4:21:13 AM10/28/19

to django-...@googlegroups.com

#30920: Choice enumeration types should not enforce enum.unique().

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Steve Jorgensen):

Makes sense. Thanks for explaining the decision in your response, @felixxm

--
Ticket URL: <https://code.djangoproject.com/ticket/30920#comment:2>

Django

unread,

Oct 28, 2019, 5:33:52 AM10/28/19

to django-...@googlegroups.com

#30920: Choice enumeration types should not enforce enum.unique().

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Nick Pope):

Hi Steve,

As Felix says, we're using enums under the hood, but we've added some
additional constraints on top.
These helper classes are intended to make working with field choices
easier and more structured.

I see the following problems with allowing duplicate values:

1. It is most likely not intended, e.g. a copy-paste error.
2. Where we use display labels, it may be unclear that multiple entries
refer to the same value.
3. Following your example, when we load the value back from the database,
is it "Default" or "Car"?

This is why we decided to enforce unique values. There is nothing to stop
someone from doing things as they did before, i.e. not using `Choices`.

For your example of a default case, I'd suggest that the blank value is
the default, e.g. set `__empty__ = _('(Default)')`.
That way you can change the default easily without needing to make a new
migration.
Alternatively, if you need to enforce the value in the database, you can
just set it in `clean()` on your form or as the `Field.default`.