Model field choices should accept empty iterator

[Daniel Naab]

The base Field class constructor uses the following shorthand to initialize the choices for a field (line 96 of django/db/model/fields/__init__.py):

self._choices = choices or []

In Python, an empty iterator evaluates to False, so if we did:

class MyModel(models.Model):
    my_field = models.IntegerField(choices=my_list_like_object)

_choices will default to an empty list if the my_list_like_object is permanently or temporarilly empty at time of class definition. However, the documentation clearly states the intent of allowing ​any iterable as a choices kwarg:

Finally, note that choices can be any iterable object -- not necessarily a list or tuple. This lets you construct choices dynamically. But if you find yourself hacking choices to be dynamic, you're probably better off using a proper database table with a ForeignKey. choices is meant for static data that doesn't change much, if ever.

So I propose making a small change to the above line:

if choices == None:
    self._choices = []
else:
    self._choices = choices

[Aymeric Augustin]

I respectfully disagree with this sentence:

In Python, an empty iterator evaluates to False

To the best of my knowledge, an iterator always evaluate to True, no matter if it's empty:

>>> bool(iter(()))
True

---

However, at first sight, the intent here is to use None instead of [] as the default value for choices, to avoid the usual mutable-default-value issue. Hence, a strict check against None looks more appropriate.

---

I began working on a patch (first patch attached)... and failed horribly.

In fact, if we want to handle properly the case when an empty list is passed in the choices argument, we mustn't set self._choices = [] when the choices argument isn't passed. Otherwise we can no longer tell if (a) no choices were given or (b) a empty list was given, that will be populated later. This becomes ugly very quickly because Django uses if <field>.choices in many places. I'm afraid a lot of third-party code (like custom fields) also uses this idiom.

In particular, Django only creates get_FOO_display when <field>.choices evaluates to True when the field is initialized, which is why the test in my patch fails.

---

So, even if we implement a strict check (self._choices = [] if choices is None else choices), it still won't be possible to use an empty list as initial value of choices. Fixing this properly is backwards-incompatible. So I recommend not changing anything :)

Since iterators always evaluate to True, this shouldn't affect too many people. We could put a note in the docs (second patch attached, it just adds the word "non-empty" and rewraps the paragraph).

[Daniel Naab]

I suspected there would be side effects of making the default self._choices == None instead of the empty list, which is why I proposed the simpler solution. A list-like object with length of zero is functionally identical to an empty list, so I don't see what possible side effects there could be to my suggestion (although setting choices to None is more sensible in an absolute sense).

[Jacob]

danielnaab, can you clarify the real-world usecase here? I'm having trouble figuring out if this is actually a bug or something more abstract. Thanks!