ModelForm validation regression in 1.2 when using TimeField with choices

[3point2]

This is my test case models.py:

from django.db import models
from django import forms


TIME_CHOICES = (
    ("09:00:00", "9 AM"),
    ("17:00:00", "5 PM")
)

class TestModel(models.Model):
    time = models.TimeField(choices=TIME_CHOICES)


class TestForm(forms.ModelForm):
    class Meta:
        model = TestModel

This is what happens with Django 1.1.4:

>>> f = TestForm({"time": "09:00:00"})
>>> f.errors
{}

and this is what happens with Django 1.2.5:

>>> f = TestForm({"time": "09:00:00"})
>>> f.errors
{'time': [u'Value datetime.time(9, 0) is not a valid choice.']}

It looks like in 1.2 the supplied time is being converted into a datetime instance before being compared to the valid choices. It would be great to see this incompatible change from 1.1 fixed or documented.

[3point2]

solution: using datetime.time instances for the choices seems to work on both 1.1 and 1.2.

[Russell Keith-Magee]

If it's a TimeField, it shouldn't be converting anything to datetimes. Since this is a regression, it's a blocker for the 1.3 release.

[3point2]

thanks for the quick reply. sorry, i didn't phrase things as precisely as i should have. when i used the word 'datetime' i was referring to the entire datetime module, not just to the datetime.datetime type.

so to put it more precisely, it looks like the ModelForm is converting the TimeField into the correct corresponding datetime.time type, but does this before comparing it to the valid choices (which are ISO time strings). the solution i'm using right now is to just use datetime.time instances in my TIME_CHOICES.

finally, i strongly suspect (but haven't tested) that the same will happen with DateFields (converted to datetime.date instances) and DateTimeFields (converted to datetime.datetime instances).

[Russell Keith-Magee]

Ok - after a closer look, this is a case of an error in your code that is now being caught.

This change was introduced by r12098 -- the introduction of model-based validation. This improved the validation of inputs across the board, including the case that you have described.

In your specific example, the error that has been caught is the fact that "09:00:00" *isn't* a valid value for a Date. It may be a valid form input, but it isn't a valid database value. As you suspect, this same change will affect other fields -- because a string isn't a valid value for a date field of any stripe.

Django's backwards compatibility guarantee doesn't apply to bugs -- it only applies to features when used correctly (or as documented). In this case, it appears that you were relying on an undocumented looseness in Django's handling of dates. r12098 improved the validation of these values, and your code has been caught in the crossfire.

[3point2]

I hope I'm not going to get on anyone's nerves by reopening this, but I think you misunderstood something: Of course "09:00:00" isn't a valid value for a Date. However, my example uses a TimeField, not a DateField. As far as I can tell (​http://docs.djangoproject.com/en/1.2/ref/forms/fields/#django.forms.TimeField.input_formats), "09:00:00" is a valid value for a time. Thoughts?

[Russell Keith-Magee]

Apologies -- I mistyped in my explanation.

No, "09:00:00" is not a valid value for a time. It's a valid *string* that can be converted into a Time value *on a form*, using the format you describe (which is, as you will note in the forms documentation). You're dealing with a Model. It's the model validation that is failing.

[3point2]

It seems like setting the time to a valid string directly on the model works fine:

>>> t = TestModel()
>>> t.time = "09:00:00"
>>> t.save()
>>>

This is what happens when I give an invalid time string:

>>> t.time = "09"
>>> t.save()
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/vasili/.virtualenvs/testing/lib/python2.6/site-packages/django/db/models/base.py", line 431, in save
    self.save_base(force_insert=force_insert, force_update=force_update)
  File "/home/vasili/.virtualenvs/testing/lib/python2.6/site-packages/django/db/models/base.py", line 495, in save_base
    rows = manager.filter(pk=pk_val)._update(values)
  File "/home/vasili/.virtualenvs/testing/lib/python2.6/site-packages/django/db/models/query.py", line 446, in _update
    query.add_update_fields(values)
  File "/home/vasili/.virtualenvs/testing/lib/python2.6/site-packages/django/db/models/sql/subqueries.py", line 249, in add_update_fields
    val = field.get_db_prep_save(val)
  File "/home/vasili/.virtualenvs/testing/lib/python2.6/site-packages/django/db/models/fields/__init__.py", line 202, in get_db_prep_save
    return self.get_db_prep_value(value)
  File "/home/vasili/.virtualenvs/testing/lib/python2.6/site-packages/django/db/models/fields/__init__.py", line 920, in get_db_prep_value
    return connection.ops.value_to_db_time(self.to_python(value))
  File "/home/vasili/.virtualenvs/testing/lib/python2.6/site-packages/django/db/models/fields/__init__.py", line 908, in to_python
    _('Enter a valid time in HH:MM[:ss[.uuuuuu]] format.'))
ValidationError: Enter a valid time in HH:MM[:ss[.uuuuuu]] format.

[Jacob]

Milestone 1.3 deleted

[guillaumechorn@…]

Replying to russellm:

Apologies -- I mistyped in my explanation.

No, "09:00:00" is not a valid value for a time. It's a valid *string* that can be converted into a Time value *on a form*, using the format you describe (which is, as you will note in the forms documentation). You're dealing with a Model. It's the model validation that is failing.

If I understand correctly, you're saying that choices for a TimeField need to be datetime.time values. However, if I set my choices like so:

TIMES = (
        (u'00:00:00',u'All Day'),
        (u'12:00:00',u'Noon')
    )

I find that while forms reject the '12:00:00' value (same error as above: 'Value datetime.time(12, 0) is not a valid choice.'), they don't reject the '00:00:00' value. This is strange. Based on the first case, it seems like what 3point2 was suggesting is happening: Django converts the '12:00:00' value from the form to datetime.time(12, 0) and then compares it to the valid choices (which are in string format), and throws an error since the two don't match up. After all, if this was simply a model validation issue, wouldn't the error message say 'Value "12:00:00" is not a valid choice.'? But then if you submit '00:00:00' on the form, no error is thrown--which doesn't agree with your explanation *or* 3point2's. So what is going on here?