Code

Opened 4 years ago

Closed 3 years ago

Last modified 3 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: UI/UX:

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@…> 4 years ago.
django_localize_filesizeformat_number.r14561.diff (4.0 KB) - added by David Danier <david.danier@…> 3 years ago.
django_localize_filesizeformat_number.r15148.diff (4.0 KB) - added by David Danier <david.danier@…> 3 years ago.

Download all attachments as: .zip

Change History (14)

Changed 4 years ago by David Danier <david.danier@…>

comment:1 Changed 4 years ago by David Danier <david.danier@…>

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 4 years ago by anonymous

  • Component changed from Uncategorized to Template system
  • Owner changed from nobody to anonymous
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 4 years ago by anonymous

  • Needs documentation set

comment:4 Changed 4 years ago by russellm

  • Needs documentation unset
  • Needs tests set

A reasonable suggestion, but the patch needs tests.

comment:5 Changed 4 years ago by David Danier <david.danier@…>

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 Changed 3 years ago by russellm

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.

Changed 3 years ago by David Danier <david.danier@…>

comment:7 Changed 3 years ago by David Danier <david.danier@…>

  • 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 Changed 3 years ago by russellm

  • Keywords blocker added

Changed 3 years ago by David Danier <david.danier@…>

comment:9 Changed 3 years ago by David Danier <david.danier@…>

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 Changed 3 years ago by russellm

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

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

comment:11 Changed 3 years ago by russellm

(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.

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.