Time-based model field cleaning and TypeErrors

[Claude Paroz]

In the case I set a wrong integer value to some time-based field (say a year as integer) and call full_clean on the instance, I'll get a TypeError as only ValueErrors are catched to produce ValidationErrors.

Should that use case be supported and should Date(Time)Field.to_python also catch TypeErrors?

[Mariusz Felisiak]

This is probably a duplicate of #21523, see comment:

The real question is the contract of the to_python method:

• Should it accept anything and never crash?

[Claude Paroz]

Yes, this is partially related to #21523, thanks for pointing it out. I checked the to_python() method of other model fields as well:

• The default to_python is just returning the passed value. The docstring say: raising django.core.exceptions.ValidationError if the data can't be converted which is likely the case in this ticket's use case.

• BooleanField.to_python is making some if tests, then raises ValidationError for any other values.

• CharField.to_python/TextField.to_python are casting to string every non-str value.

• DecimalField.to_python is catching TypeError when trying to cast to Decimal.

• FloatField.to_python is catching TypeError when trying to cast to float.

• IntegerField.to_python is catching TypeError when trying to cast to int.

• GenericIPAddressField.to_python is casting values to str (aka CharField).

• BinaryField.to_python is just returning the value if not a str.

• UUIDField.to_python is catching AttributeError which will be returned for any non-supported types (because of calling .replace on the value).

In the custom model fields documentation, we can read: For to_python(), if anything goes wrong during value conversion, you should raise a ValidationError exception.

So after reviewing different to_python implementations, there is clearly a discrepancy between date-related fields (including DurationField) and all other model fields. And whatever I said in the past, today I would clearly answer Yes to the question of should-not-crash.

I'm still commenting here, as #21523 is a bit different, as solving this (that is catch TypeError would still not solve their use case).

[Mariusz Felisiak]

Replying to Claude Paroz:

I'm still commenting here, as #21523 is a bit different, as solving this (that is catch TypeError would still not solve their use case).

It should solve #21577 which was marked as a duplicate of #21523 :)