#15789 closed Bug (fixed)
floatformat filter works incorrectly when decimal precision is set low
Reported by: | Antti Kaihola | Owned by: | Jose Ignacio Galarza |
---|---|---|---|
Component: | Template system | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | eire1130@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
When floatformat
receives a decimal places argument which is equal or larger than the current decimal precision, the decimal part isn't truncated. An example:
>>> from django.template.defaultfilters import floatformat >>> import decimal >>> decimal.getcontext().prec = 2 >>> floatformat(1.23456789123, 1) u'1.2' >>> floatformat(1.23456789123, 2) u'1.23456789123' >>> floatformat(1.23456789123, 3) u'1.23456789123'
Attachments (4)
Change History (20)
comment:1 by , 14 years ago
comment:2 by , 14 years ago
Easy pickings: | set |
---|---|
Needs tests: | set |
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 14 years ago
Has patch: | set |
---|---|
Needs tests: | unset |
Owner: | changed from | to
Status: | new → assigned |
Hi,
I found some more problems. For example, with a precission of two digits, this regression test fails:
self.assertEqual(floatformat(13.1031, -3), u'13.103')
I think this patch solves these problems.
comment:4 by , 13 years ago
UI/UX: | unset |
---|
The current patch no longer applies cleanly to trunk. I tried updating the patch, believe I updated it accurately, but still had test failures
FAIL: test_l10n_disabled (regressiontests.i18n.tests.FormattingTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/preston/Projects/code/forks/django/tests/regressiontests/i18n/tests.py", line 226, in test_l10n_disabled self.assertEqual(u'66666.67', Template('{{ n|floatformat:2 }}').render(self.ctxt)) AssertionError: u'66666.67' != u'66666.666' ====================================================================== FAIL: test_l10n_enabled (regressiontests.i18n.tests.FormattingTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/preston/Projects/code/forks/django/tests/regressiontests/i18n/tests.py", line 320, in test_l10n_enabled self.assertEqual(u'66666,67', Template('{{ n|floatformat:2 }}').render(self.ctxt)) AssertionError: u'66666,67' != u'66666.666'
Sorry - kicking the can....
comment:5 by , 13 years ago
I applied my patch to the trunk, and I've sent a pull request on github: https://github.com/django/django/pull/44
comment:6 by , 13 years ago
Patch needs improvement: | set |
---|
The current pull applies cleanly - but still fails tests and doing some quick experiments appears to fail to perform - perhaps it is in the absence of a decimal context precision set explicitly
>>> from django.template.defaultfilters import floatformat >>> import decimal >>> x = decimal.Decimal('5555.555') >>> y = 5555.555 >>> floatformat(x,2) u'5555.555' >>> floatformat(y,2) u'5555.555' >>>
by , 13 years ago
Attachment: | patch.2.diff added |
---|
comment:7 by , 13 years ago
Yes, you are right!
I resubmit the patch (patch2.diff), what do you think? :)
comment:8 by , 13 years ago
Needs tests: | set |
---|
It might be nice to add a test for the decimal type since you already have decimal imported:
Not stopper, but since you seem to be responsive currently for this ticket, would you mind adding?
self.assertEqual(floatformat(decimal.Decimal('5555.555', 2), u'5555.56')
Otherwise this is looking good.
comment:9 by , 13 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
Of course!
I added more test funcionts in the last patch (about the decimal module).
Thank you very much for your review :)
comment:10 by , 13 years ago
milestone: | → 1.4 |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Latest patch look good
comment:12 by , 13 years ago
I'm going to improve a little bit the patch before committing it. In particular decimal.getcontext().prec
should be restored at the end of the test.
comment:14 by , 13 years ago
Cc: | added |
---|---|
Version: | master → 1.3 |
Can a commiter send this patch to 1.3? It's in 1.4, but it would be helpful to have this in 1.3
comment:15 by , 13 years ago
Only security fixes are backported to the 1.3.X branch.
See this page for more information concerning backport policy.
comment:16 by , 13 years ago
A quick correction: actually, only security fixes are applied to 1.3.X. I got that wrong, too: see #18135.
What happens inside
floatformat
when evaluatingdecimal.getcontext().prec = 2 ; floatformat(1.23456789123, 2)
is equivalent to this:At this point,
floatformat
eats the exception and just returns the original value without truncating the decimal part.