#35852: intcomma not locale aware when given a decimal as string
-------------------------------------+-------------------------------------
Reporter: Jonathan Ströbele | Owner: HEMANT
| MISHRA
Type: Bug | Status: assigned
Component: contrib.humanize | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jonathan Ströbele):
Hi Natalia,
to be honest I'm not quite sure what the best fix actually would be and I
need a bit more guidance here. ;) As I mentioned I think there is a mis-
match between the documentation and the actual code, and DocBlock. Also
I'm not sure what `intcomma` actually was intended to mean and how it
should be used:
1. `int[eger]comma` as in separate a integer into groups of 3 digits
separated by a comma (only valid for an `en` locale). This seems to be the
[https://github.com/django/django/commit/fb537e177d7d304d6642ee6005258a82584a8032
original code and intention] of the function when it was added to Django.
2. `int[ernational]comma` as in provide a manual way to print a grouped
number according to the current locale, if the setting
`USE_THOUSAND_SEPARATOR` is `False` and so no automatic number grouping is
happening.
The 2. option is what I figured from reading the documentation and also
think that's the intended function from the current state of the source
code. Though it has the aforementioned bug regarding the floats passed as
string where it's using a fallback to the old logic that's correct in the
`en` locale.
Since `intcomma` is passing down the given value to the
`django.utils.formats.number_format` (with enabled `force_grouping=True`)
if it's a float or Decimal already, and only uses the code that is `en`
locale specific code in case of a string, I would suggest to use
`intcomma` as a wrapper of `number_format` with `force_grouping=True`.
Since the `number_format` already handles the formatting/grouping of
numbers, I think it's no longer needed to reproduce this logic in
`intcomma`?
Making this change seems to conform to nearly all unit tests in
`tests/humanize_tests/tests.py`, except for some unicode stuff introduced
in #28628 (fe76944269c13d59f8bb3bc1cfff7ab6443777e4) and a long string
without numbers (16a8fe18a3b81250f4fa57e3f93f0599dc4895bc).
This seems to be the case, because `number_format` doesn't check if the
string is containing digits, and just inserts the thousand seperator into
the string. Which I think is Okay?
{{{
AssertionError: '１,２３４,５６７' != '1,234,567' : intcomma test failed,
produced '１,２３４,５６７', should've produced '1,234,567'
AssertionError: 'th,e q,uic,k b,row,n f,ox ,jum,ped, ov,er ,the, la,zy
,dog' != 'the quick brown fox jumped over the lazy dog' : intcomma test
failed, produced 'th,e q,uic,k b,row,n f,ox ,jum,ped, ov,er ,the, la,zy
,dog', should've produced 'the quick brown fox jumped over the lazy dog'
}}}
I have created a suggestion for a patch in a fork here:
https://github.com/stroebjo/django/commit/f5302d43c45208ebb92516ab2724e86a1e60d71d
--
Ticket URL: <https://code.djangoproject.com/ticket/35852#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/01070192bb31c7eb-f76f505f-4a98-4433-8ec9-1f5a4839d4fe-000000%40eu-central-1.amazonses.com.
