[Django] #31841: DecimalField doesnt't respect max_digits when converting
Django
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
bellini666 |
Type: Bug | Status: new
Component: Database | Version: 3.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
A field defined like this:
{{{
foo = models.DecimalField(max_digits=20, decimal_places=6)
}}}
When trying to set a float 0.2 it would give an error regarding the
maximum decimal places. That because the context that converts the float
to decimal only respect max_digits:
{{{
In [1]: import decimal
In [2]: c = decimal.Context(prec=20)
In [3]: c.create_decimal_from_float(0.2)
Out[3]: Decimal('0.20000000000000001110')
}}}
For floats django should probably round the value to the decimal_places
defined in the field.
--
Ticket URL: <https://code.djangoproject.com/ticket/31841>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Ribeiro |
Type: Bug | Status: new
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Thiago Bellini Ribeiro):
Just created a PR that fix this issue:
https://github.com/django/django/pull/13258
--
Ticket URL: <https://code.djangoproject.com/ticket/31841#comment:1>
Django
-------------------------------------+-------------------------------------
Ribeiro |
Component: Database layer | Version: 3.0
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
Thanks for this ticket, however using `round()` is not an approach that
will work in all cases, see [https://groups.google.com/g/django-
developers/c/bnoVTOx2GFs/m/i0lNDpV8EgAJ discussion] and #28164. That's why
we decided that providing a custom context to `DecimalField` is a proper
way to fix this and similar issues, see #26459.
--
Ticket URL: <https://code.djangoproject.com/ticket/31841#comment:2>
Django
-------------------------------------+-------------------------------------
Ribeiro |
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Thiago Bellini Ribeiro):
Hi felixxm,
I understand what you mean regarding the context but I don't think the the
proposed solution to the issue there can solve this one here.
The context itself can't define the number of decimal places, just the
precision that is the number of digits in the float. So, there's no way to
configure the context so it will respect DecimalField's max_digits.
The example I gave above is a perfect example to that and something that
happened to me some times in some of my project. Even if the field defined
decimal_places of 6 and since it has 20 max_digits defined, the code would
convert 0.2 to something that has 19 decimal places. That exact code and
the test cases that I changed there would give errors if they were running
really being set on a database with postgresql (don't know if other
databases would produce errors as well).
If the round doesn't work for everything maybe we can explore other
solutions, but since the DecimalField allows for float conversion and used
a context to make sure it respects max_digits, IMHO it should also respect
decimal_places too, or else you end up with corner cases like that where
you can't trust the float conversion ever.
--
Ticket URL: <https://code.djangoproject.com/ticket/31841#comment:3>
Django
-------------------------------------+-------------------------------------
Ribeiro |
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* resolution: wontfix => duplicate
Comment:
We could use `quantize()` that accepts `context`, e.g.
{{{
v = self.context.create_decimal_from_float(value)
quantize_value = decimal.Decimal(1).scaleb(-self.decimal_places)
return v.quantize(quantize_value, context=self.context)
}}}
but first we need to support a custom context (#26459), because the
default behavior may not be appropriate for all users. I would treat this
as a part of #26459.
--
Ticket URL: <https://code.djangoproject.com/ticket/31841#comment:4>