Skip to content

Loading…

Enable the use of any two item iterable within an iterable for choices #1081

Merged
merged 1 commit into from

3 participants

@dstufft
Django member

Small change that enables the use of non list/tuple but still iterable two item objects for the model choices.

@charettes
Django member

What about using a collections.Iterable instance check?

@dstufft
Django member

@charettes Existing code used django.utils.itercompat.is_iterable() and I did the same in order to maintain consistency.

@charettes
Django member

I guess the removal/deprecation of is_iterable() is another issue, thanks for pointing this out.

Since this will need additional tests and might be worth a release note could you open a Trac ticket and link it to this PR?

@dstufft
Django member

You want a ticket and such for the remova/deprecation of is_iterable()? I can do that.

Is there anything else you see about this ticket?

@charettes
Django member

Sorry I meant a ticket for this PR. I can do the is_iterable() one.

This ticket will need tests with a non list, tuple choice.

@dstufft
Django member

@charettes Do you have an opinion on where these tests should live? As far as I can tell none of the validation is tested other than invalid cases.

@dstufft
Django member

Ok! Tests and documentation written.

@ptone ptone commented on an outdated diff
docs/ref/models/fields.txt
@@ -80,9 +80,9 @@ If a field has ``blank=False``, the field will be required.
.. attribute:: Field.choices
-An iterable (e.g., a list or tuple) of 2-tuples to use as choices for this
-field. If this is given, the default form widget will be a select box with
-these choices instead of the standard text field.
+An iterable (e.g., a list or tuple) of 2 item iterables (e.g. tuple) to use as
@ptone Django member
ptone added a note

This becomes a bit of a mouthful - maybe:

An iterable (e.g., a list or tuple) consisting itself of iterables of exactly two items. ((A, B), (A, B)...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ptone ptone commented on the diff
tests/model_validation/models.py
((8 lines not shown))
+ self.display = display
+
+ def __iter__(self):
+ return (x for x in [self.value, self.display])
+
+ def __len__(self):
+ return 2
+
+
+class Things(object):
+
+ def __iter__(self):
+ return (x for x in [ThingItem(1, 2), ThingItem(3, 4)])
+
+
+class ThingWithIterableChoices(models.Model):
@ptone Django member
ptone added a note

I'd add a comment with the ticket number here and in test below for posterity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ptone ptone commented on an outdated diff
tests/model_validation/tests.py
@@ -0,0 +1,11 @@
+import io
+
+from django.core import management
+from django.test import TestCase
+
+
+class ModelValidationTest(TestCase):
+
+ def test_models_validate(self):
+ # All our models should validate properly
+ management.call_command("validate", stdout=io.BytesIO())
@ptone Django member
ptone added a note

does this actually raise an error if validation fails? - I seem to recall that errors are just printed to stdout

I know you are pushing into new testing areas here (thanks for going the extra distance BTW) but a failing test would be nice to at least demonstrate this testing approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dstufft dstufft Fixed #20430 - Enable iterable of iterables for model choices
Allows for any iterable, not just lists or tuples, to be used as
the inner item for a list of choices in a model.
a19e9d8
@dstufft dstufft merged commit a19e9d8 into django:master
@dstufft dstufft deleted the dstufft:allow-any-iterable-for-choices branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 18, 2013
  1. @dstufft

    Fixed #20430 - Enable iterable of iterables for model choices

    dstufft committed
    Allows for any iterable, not just lists or tuples, to be used as
    the inner item for a list of choices in a model.
View
4 django/core/management/validation.py
@@ -118,8 +118,8 @@ def get_validation_errors(outfile, app=None):
e.add(opts, '"%s": "choices" should be iterable (e.g., a tuple or list).' % f.name)
else:
for c in f.choices:
- if not isinstance(c, (list, tuple)) or len(c) != 2:
- e.add(opts, '"%s": "choices" should be a sequence of two-tuples.' % f.name)
+ if isinstance(c, six.string_types) or not is_iterable(c) or len(c) != 2:
+ e.add(opts, '"%s": "choices" should be a sequence of two-item iterables (e.g. list of 2 item tuples).' % f.name)
if f.db_index not in (None, True, False):
e.add(opts, '"%s": "db_index" should be either None, True or False.' % f.name)
View
7 docs/ref/models/fields.txt
@@ -80,9 +80,10 @@ If a field has ``blank=False``, the field will be required.
.. attribute:: Field.choices
-An iterable (e.g., a list or tuple) of 2-tuples to use as choices for this
-field. If this is given, the default form widget will be a select box with
-these choices instead of the standard text field.
+An iterable (e.g., a list or tuple) consisting itself of iterables of exactly
+two items (e.g. ``[(A, B), (A, B) ...]``) to use as choices for this field. If
+this is given, the default form widget will be a select box with these choices
+instead of the standard text field.
The first element in each tuple is the actual value to be stored, and the
second element is the human-readable name. For example::
View
3 docs/releases/1.6.txt
@@ -238,6 +238,9 @@ Minor features
Meta option: ``localized_fields``. Fields included in this list will be localized
(by setting ``localize`` on the form field).
+* The ``choices`` argument to model fields now accepts an iterable of iterables
+ instead of requiring an iterable of lists or tuples.
+
Backwards incompatible changes in 1.6
=====================================
View
4 tests/invalid_models/invalid_models/models.py
@@ -375,8 +375,8 @@ class Meta:
invalid_models.fielderrors: "decimalfield4": DecimalFields require a "max_digits" attribute value that is greater than or equal to the value of the "decimal_places" attribute.
invalid_models.fielderrors: "filefield": FileFields require an "upload_to" attribute.
invalid_models.fielderrors: "choices": "choices" should be iterable (e.g., a tuple or list).
-invalid_models.fielderrors: "choices2": "choices" should be a sequence of two-tuples.
-invalid_models.fielderrors: "choices2": "choices" should be a sequence of two-tuples.
+invalid_models.fielderrors: "choices2": "choices" should be a sequence of two-item iterables (e.g. list of 2 item tuples).
+invalid_models.fielderrors: "choices2": "choices" should be a sequence of two-item iterables (e.g. list of 2 item tuples).
invalid_models.fielderrors: "index": "db_index" should be either None, True or False.
invalid_models.fielderrors: "field_": Field names cannot end with underscores, because this would lead to ambiguous queryset filters.
invalid_models.fielderrors: "nullbool": BooleanFields do not accept null values. Use a NullBooleanField instead.
View
0 tests/model_validation/__init__.py
No changes.
View
27 tests/model_validation/models.py
@@ -0,0 +1,27 @@
+from django.db import models
+
+
+class ThingItem(object):
+
+ def __init__(self, value, display):
+ self.value = value
+ self.display = display
+
+ def __iter__(self):
+ return (x for x in [self.value, self.display])
+
+ def __len__(self):
+ return 2
+
+
+class Things(object):
+
+ def __iter__(self):
+ return (x for x in [ThingItem(1, 2), ThingItem(3, 4)])
+
+
+class ThingWithIterableChoices(models.Model):
@ptone Django member
ptone added a note

I'd add a comment with the ticket number here and in test below for posterity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ # Testing choices= Iterable of Iterables
+ # See: https://code.djangoproject.com/ticket/20430
+ thing = models.CharField(max_length=100, blank=True, choices=Things())
View
14 tests/model_validation/tests.py
@@ -0,0 +1,14 @@
+import io
+
+from django.core import management
+from django.test import TestCase
+
+
+class ModelValidationTest(TestCase):
+
+ def test_models_validate(self):
+ # All our models should validate properly
+ # Validation Tests:
+ # * choices= Iterable of Iterables
+ # See: https://code.djangoproject.com/ticket/20430
+ management.call_command("validate", stdout=io.BytesIO())
Something went wrong with that request. Please try again.