forms.BooleanField field issue

[george.li]

in django 1.4 1.5.1

from django import forms
forms.BooleanField().clean(True)

this case is success

But

forms.BooleanField().clean(False)

forms.BooleanField().clean('false')

in this two case are
* ValidationError: [u'This field is required.']

source code

 621 class BooleanField(Field):
 622     widget = CheckboxInput
 623
 624     def to_python(self, value):
 625     ¦   """Returns a Python boolean object."""
 626     ¦   # Explicitly check for the string 'False', which is what a hidden field
 627     ¦   # will submit for False. Also check for '0', since this is what
 628     ¦   # RadioSelect will provide. Because bool("True") == bool('1') == True,
 629     ¦   # we don't need to handle that explicitly.
 630     ¦   if isinstance(value, six.string_types) and value.lower() in ('false', '0'):
 631     ¦   ¦   value = False
 632     ¦   else:
 633     ¦   ¦   value = bool(value)
 634     ¦   value = super(BooleanField, self).to_python(value)
 635     ¦   if not value and self.required:
 636     ¦   ¦   raise ValidationError(self.error_messages['required'])
 637     ¦   return value

in 630, 'false' and '0' Transform to boolean False
but 635 then decide False is no input value

[Aymeric Augustin]

If you want the boolean field not to be required, you must create it with the required=False option.

[Aryeh Hillman]

In django 1.7, this is an issue. There may have been some regression since this ticket was marked worksforme.

>>> from django import forms
>>> forms.BooleanField().clean(True)
True
>>> forms.BooleanField().clean(False)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Library/Python/2.7/site-packages/django/forms/fields.py", line 151, in clean
    self.validate(value)
  File "/Library/Python/2.7/site-packages/django/forms/fields.py", line 735, in validate
    raise ValidationError(self.error_messages['required'], code='required')
ValidationError: [u'This field is required.']

The bug is in the validate method on forms.BooleanField.

It's code is:

    def validate(self, value):
        if not value and self.required:
            raise ValidationError(self.error_messages['required'], code='required')

On forms.Field, however, there is a class variable, empty_values that should be used. That is, the correct implementation would be:

    def validate(self, value):
        if value in self.empty_values and self.required:
            raise ValidationError(self.error_messages['required'], code='required')

Thanks!

[Tim Graham]

Looking at your example, the field is working as expected. As noted in comment 1, if you want to accept False, you need to use required=False. The ​docs for BooleanField describe the behavior.

[Aryeh Hillman]

timgraham. This is not the correct implementation. If you set required=False, then supplying an empty value will validate as will False. I understand that there could be a backwards compatibility issue here, which is fine. But it seems to me that this is almost definitely the wrong implementation.

[Aryeh Hillman]

It seems to me that the original thinking on this was the following: making a boolean field required is good for cases where a user must, say, check a box to accept terms of service. This seems to me, however, that it should be handled at the form-level clean or with a custom validator on the field — not by setting required=False. Another argument is that BooleanField is one of the only fields that overrides forms.Field's validate method. Whereas the other fields rely on self.empty_values, forms.BooleanField is the only field that relies on the provided value's implicit boolean value. Definitely inconsistent.

But again, I understand completely if this is a backwards compatibility case. It just is terribly confusing in many cases, especially when wanting to, say, use forms for validating some JSON data. Say you always want a boolean field to show up in the data, then, really, the best way to deal with this situation is to sub-class BooleanField and override its validate method. Or write a custom validator — but its a bit hacky to circularly reference the calling class' empty_values from an outside method.

Again, I understand your reasoning to some degree. It IS in fact documented, if otherwise confusing.