Opened 14 years ago

Closed 13 years ago

Last modified 13 years ago

#14240 closed (fixed)

filesizeformat should localize number

Reported by: David Danier <david.danier@…> Owned by: anonymous
Component: Template system Version: 1.2
Severity: Keywords: blocker
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently filesizeformat always displays file size in english number formating. But as we now have formats.number_format() I think this should be used. Possible example:

In [1]: from django.template.defaultfilters import filesizeformat

In [2]: filesizeformat(100)
Out[2]: u'100 bytes'

In [3]: filesizeformat(1000)
Out[3]: u'1000 bytes'

In [4]: filesizeformat(2000)
Out[4]: u'1.9 KB'

In [5]: filesizeformat(15000)
Out[5]: u'14.6 KB'

In [6]: from django.utils import translation

In [7]: translation.activate('de')

In [8]: filesizeformat(15000)
Out[8]: u'14,6 KB'

In [9]: filesizeformat(1500000)
Out[9]: u'1,4 MB'

In [10]: filesizeformat(1500000000)
Out[10]: u'1,3 GB'

In [11]: filesizeformat(1500000000000)
Out[11]: u'1,3 TB'

Patch follows.

Attachments (3)

django_localize_filesizeformat_number.diff (1.3 KB ) - added by David Danier <david.danier@…> 14 years ago.
django_localize_filesizeformat_number.r14561.diff (4.0 KB ) - added by David Danier <david.danier@…> 13 years ago.
django_localize_filesizeformat_number.r15148.diff (4.0 KB ) - added by David Danier <david.danier@…> 13 years ago.

Download all attachments as: .zip

Change History (14)

by David Danier <david.danier@…>, 14 years ago

comment:1 by David Danier <david.danier@…>, 14 years ago

Has patch: set

comment:2 by anonymous, 14 years ago

Component: UncategorizedTemplate system
Owner: changed from nobody to anonymous
Status: newassigned
Triage Stage: UnreviewedAccepted

comment:3 by anonymous, 14 years ago

Needs documentation: set

comment:4 by Russell Keith-Magee, 13 years ago

Needs documentation: unset
Needs tests: set

A reasonable suggestion, but the patch needs tests.

comment:5 by David Danier <david.danier@…>, 13 years ago

I'm not sure how to write these. Can you point me into the right direction? My first attempt on putting tests into regressiontests.defaultfilters seems to have failed because USE_I18N and/or USE_L10N is set to False. Then I discovered regressiontests.i18n, but there are no filter-tests in there (or at least I haven't found them).

Apart from that: It seems like formats.number_format() just strips of the last digits if the float/decimal has to much decimal places. So some existing tests fail using the above patch as 1023.999something will result in "1023,9" instead of "1024.0". Is this the intended behavior of formats.number_format()? If so I will try to update the patch.

comment:6 by Russell Keith-Magee, 13 years ago

The rc1 patch on #14181 should give you an idea of how to handle testing of this feature.

What you describe certainly sounds like a problem; however, it should be tracked as a separate ticket.

by David Danier <david.danier@…>, 13 years ago

comment:7 by David Danier <david.danier@…>, 13 years ago

Needs tests: unset

I attached an updated patch including:

  • Use of round() in filesizeformat(). This should be ok, as all numbers are converted to float first, so we don't need to handle Decimal.quantize().
  • Fix of fallback-output, which is currently not localized ("0 bytes", see except, 0 might be singular in some languages or something totally different, who knows)
  • Added tests, thanks Russel for the pointer.
  • Updated myself in AUTHORS

About round(): I think just cutting of the last digits inside number_format() should be the intended behaviour. number_format() should not care about doing something other than just formatting the number. So round() must be called outside of number_format(). Perhaps Django could provide some generic round()-function to accomplish differences betweeen float and Decimal, but thats definately outside the scope of this ticket and number_format.

comment:8 by Russell Keith-Magee, 13 years ago

Keywords: blocker added

by David Danier <david.danier@…>, 13 years ago

comment:9 by David Danier <david.danier@…>, 13 years ago

Updated patch to current revision.

In addition I noticed, that my fallback-output-fix used the variable bytes when calling ungettext, which of course looks like nonsense. This is changed now.

comment:10 by Russell Keith-Magee, 13 years ago

Resolution: fixed
Status: assignedclosed

(In [15290]) Fixed #14240 -- Enabled localization for the filesize filter. Thanks to David Danier for the report and patch.

comment:11 by Russell Keith-Magee, 13 years ago

(In [15291]) [1.2.X] Fixed #14240 -- Enabled localization for the filesize filter. Thanks to David Danier for the report and patch.

Backport of r15290 from trunk.

Note: See TracTickets for help on using tickets.
Back to Top