#34557: Time-based model field cleaning and TypeErrors
-------------------------------------+-------------------------------------
Reporter: Claude Paroz | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by 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).
--
Ticket URL: <https://code.djangoproject.com/ticket/34557#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/010701880bb24cb6-61c3fd0a-a71a-4e47-b7d7-c1a6529add9b-000000%40eu-central-1.amazonses.com.
