Code

Opened 7 years ago

Closed 7 years ago

#3004 closed enhancement (fixed)

[patch] add precision argument to floatformat from default filters

Reported by: real.human@… Owned by: adrian
Component: Template system Version: master
Severity: normal Keywords: floatformat precision
Cc: real.human@…, gary.wilson@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

updated floatformat and related docs to allow for a precision argument (which defaults to 1).

Attachments (6)

floatformat.diff (1.8 KB) - added by real.human@… 7 years ago.
floatformat.patch (2.5 KB) - added by SmileyChris 7 years ago.
Updated patch, with tests
floatformat.2.diff (1.8 KB) - added by real.human@… 7 years ago.
floatformat.3.diff (2.2 KB) - added by real.human@… 7 years ago.
floatformat.4.diff (2.4 KB) - added by real.human@… 7 years ago.
floatformat.5.diff (2.8 KB) - added by mrmachine <real.human@…> 7 years ago.

Download all attachments as: .zip

Change History (22)

Changed 7 years ago by real.human@…

comment:1 Changed 7 years ago by anonymous

  • Cc real.human@… added

comment:2 Changed 7 years ago by SmileyChris

The following line is incorrect:

return re.sub(r'\.?0+$', '', ('%%.%sf' % precision) % f)

With both precision and f set to 0, you'll return '' instead of '0'

Changed 7 years ago by SmileyChris

Updated patch, with tests

comment:3 Changed 7 years ago by SmileyChris

Ok, my patch fixes the above issue, but I noticed when running against existing tests that this patch changes the existing behaviour.

floatformat(0.007) used to return 0.0, now returns 0. I'm not sure if the original test is correct - if it is, this patch will need to be modified.

Changed 7 years ago by real.human@…

comment:4 Changed 7 years ago by real.human@…

whoops, i just updated my original patch, too (floatformat.2.diff). i also noticed a rounding error in the old version, so now i'm using str() and round() and then stripping trailing periods and zeroes.

comment:5 Changed 7 years ago by SmileyChris

Yep, that's even a better solution :)

Put my tests in your diff too. My comments still stand about this changing existing behaviour.

I also noticed rounding errors using string %s but they still came up for me using str(round( (eg: str(round(1.015, 2)))

Changed 7 years ago by real.human@…

comment:6 Changed 7 years ago by real.human@…

floatformat.3.diff includes your regression test changes.

i also noticed that in the old behaviour, floatformat(0.0) would return '0', but floatformat(0.007) would return '0.0'. as the behaviour so far has been to strip trailing zeroes, i think we should also to strip trailing periods to remove this inconsistency. if anyone wants to keep trailing periods and zeroes, they can use the stringformat filter.

about the rounding error, the old behaviour which uses python's string formatting rounds up for .6 but down for .5, while round() goes up for .5 and down for .4, which i believe is correct. if that's true, is this a bug in python's string formatting?

>>> '%.1f' % 0.15
'0.1'
>>> '%.1f' % 0.16
'0.2'
>>> round(0.14, 1)
0.10000000000000001
>>> round(0.15, 1)
0.20000000000000001
>>> str(round(0.14, 1))
'0.1'
>>> str(round(0.15, 1))
'0.2'

comment:7 Changed 7 years ago by SmileyChris

Yes, it's a rounding error in Python's string formatting. Like I said in my example, str(round()) doesn't fix everything:

>>> str(round(1.015, 2))
'1.01'

comment:8 Changed 7 years ago by SmileyChris

And you still haven't included the new tests in your diff!

Changed 7 years ago by real.human@…

Changed 7 years ago by mrmachine <real.human@…>

comment:9 Changed 7 years ago by mrmachine <real.human@…>

6th time's a charm. tests should be in the diff now, and i've used the decimal module to round the input accurately.

comment:10 Changed 7 years ago by Gary Wilson <gary.wilson@…>

  • Cc gary.wilson@… added

comment:11 Changed 7 years ago by SmileyChris

Hate to tell you this, but decimal is new in 2.4. Django needs Python 2.3 compatibility.

comment:12 Changed 7 years ago by mrmachine <real dot human at mrmachine dot net>

bummer. well, we can just use floatformat.4.diff and live with the rounding errors (unless there is another alternative).

comment:13 Changed 7 years ago by Eric Floehr <eric@…>

Created ticket #3176 as an alternative implementation to this.

comment:14 Changed 7 years ago by SmileyChris

  • Resolution set to fixed
  • Status changed from new to closed

Implemented in [4274]

comment:15 Changed 7 years ago by Mark Riedesel <mriedesel@…>

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version set to SVN

I see this bug was fixed almost a year ago, but it seems like either it's broken again or I'm using it improperly. The value I'm passing to the filter is 18.125, if I use floatformat:2 the result is 18.12 instead of the expected 18.13. Tested with SVN revision 6483 (Sept 13th, 2007)

comment:16 Changed 7 years ago by SmileyChris

  • Resolution set to fixed
  • Status changed from reopened to closed

This ticket was specifically to introduce a precision argument and that hasn't broken.

The bug you're seeing is a Python rounding problem with float, but we can get around it by using decimal. I opened a new ticket (#5748) now a backwards compatible decimal has been added to Django.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.