Opened 2 years ago
Last modified 2 years ago
#34260 closed Cleanup/optimization
models.FloatField documentation doesn't mention that +inf, -inf, and NaN are database-dependent. — at Version 5
Reported by: | Matt Cooper | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | 4.1 |
Severity: | Normal | Keywords: | floatfield |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
The model field reference documentation doesn't mention that, in a FloatField, non-numeric values have database-dependent behavior. float('nan')
on SQLite requires the field to be nullable, for example. I saw old discussion about why this is the case, but only after being bitten by it in a project. I'd like to submit a patch adding this info to the docs.
Change History (5)
comment:1 by , 2 years ago
comment:2 by , 2 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
Summary: | Field reference documentation doesn't mention that +inf, -inf, and NaN are invalid → models.FloatField documentation doesn't mention that +inf, -inf, and NaN are invalid. |
Storing inf
, -inf
, and nan
in FloatField
works for me, I don't see any protection against them in models.FloatField
:
-
tests/model_fields/test_floatfield.py
diff --git a/tests/model_fields/test_floatfield.py b/tests/model_fields/test_floatfield.py index 264b5677e9..e3b3fde380 100644
a b 1 import math 2 1 3 from django.db import transaction 2 4 from django.test import TestCase 3 5 … … class TestFloatField(TestCase): 31 33 with self.assertRaisesMessage(TypeError, msg): 32 34 obj.save() 33 35 36 def test_save_inf(self): 37 for value in [float("inf"), math.inf, "inf"]: 38 FloatModel.objects.create(size=value) 39 for value in [float("-inf"), -math.inf, "-inf"]: 40 FloatModel.objects.create(size=value) 41 42 def test_save_nan(self): 43 for value in [float("nan"), math.nan, "nan"]: 44 FloatModel.objects.create(size=value) 45 34 46 def test_invalid_value(self): 35 47 tests = [ 36 48 (TypeError, ()),
comment:3 by , 2 years ago
I suppose it would make sense to add the above tests. It'd probably also make sense to round-trip the value to make sure that it is actually supported...
It's also curious that b92ffebb0cdc469baaf1b8f0e72dddb069eb2fb4 (#33954) explicitly made this unsupported where (at least) PostgreSQL does support nan
, +inf
, and -inf
. Arguably I'd say that that was a backward incompatible change with no release note and people may already be depending on that support. Lack of support for these values is a database-specific thing.
comment:4 by , 2 years ago
Resolution: | invalid |
---|---|
Status: | closed → new |
Ah, I made two mistakes in my research, covered below.
I believe a scoped-down version of this proposed change is valid, because float('nan')
is considered "null" (at least on SQLite). This is not obvious, and much like the "empty string vs null" discussion in CharField, would have helped me. The infinities do seem to work properly, so as mentioned above, this must be database-dependent.
In that test case, is the field null=True
? In a brand new Django 4.1 project with the following models.py
pointing to a SQLite database:
from django.db import models class Example(models.Model): nullable_float = models.FloatField(null=True) nonnullable_float = models.FloatField()
then I get the following in ./manage.py shell
:
>>> from repro.models import Example >>> e1 = Example(nullable_float=1.0, nonnullable_float=2.0) >>> e2 = Example(nullable_float=float('nan'), nonnullable_float=2.0) >>> e3 = Example(nullable_float=1.0, nonnullable_float=float('nan')) >>> e1.save() >>> e2.save() >>> e3.save() Traceback (most recent call last): File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py", line 89, in _execute return self.cursor.execute(sql, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/backends/sqlite3/base.py", line 357, in execute return Database.Cursor.execute(self, query, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ sqlite3.IntegrityError: NOT NULL constraint failed: repro_example.nonnullable_float The above exception was the direct cause of the following exception: Traceback (most recent call last): File "<console>", line 1, in <module> File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/models/base.py", line 812, in save self.save_base( File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/models/base.py", line 863, in save_base updated = self._save_table( ^^^^^^^^^^^^^^^^^ File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/models/base.py", line 1006, in _save_table results = self._do_insert( ^^^^^^^^^^^^^^^^ File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/models/base.py", line 1047, in _do_insert return manager._insert( ^^^^^^^^^^^^^^^^ File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/models/manager.py", line 85, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/models/query.py", line 1791, in _insert return query.get_compiler(using=using).execute_sql(returning_fields) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 1660, in execute_sql cursor.execute(sql, params) File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py", line 103, in execute return super().execute(sql, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py", line 67, in execute return self._execute_with_wrappers( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers return executor(sql, params, many, context) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py", line 84, in _execute with self.db.wrap_database_errors: File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/utils.py", line 91, in __exit__ raise dj_exc_value.with_traceback(traceback) from exc_value File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py", line 89, in _execute return self.cursor.execute(sql, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/vtbassmatt/Library/Caches/pypoetry/virtualenvs/django34260-KHbD0UBT-py3.11/lib/python3.11/site-packages/django/db/backends/sqlite3/base.py", line 357, in execute return Database.Cursor.execute(self, query, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ django.db.utils.IntegrityError: NOT NULL constraint failed: repro_example.nonnullable_float
And for completeness, my mistakes:
- I was looking at the
FloatField
code for forms, not models, because I searchedFloatField
and didn't pay close enough attention. See https://github.com/django/django/blob/0fbdb9784da915fce5dcc1fe82bac9b4785749e5/django/forms/fields.py#L370. - I had only tested
float('nan')
myself. Once I made the above search error, I assumed thatfloat('inf')
would behave identically, but didn't test it since I wanted to get back to my task at hand.
comment:5 by , 2 years ago
Description: | modified (diff) |
---|---|
Summary: | models.FloatField documentation doesn't mention that +inf, -inf, and NaN are invalid. → models.FloatField documentation doesn't mention that +inf, -inf, and NaN are database-dependent. |
https://github.com/django/django/pull/16455 proposes a fix for the dev documentation. This change could (should?) also be backported to all supported Django versions.