Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Internationalise the decimal separator in intcomma() #53

Merged
merged 1 commit into from Aug 30, 2022
Merged

Internationalise the decimal separator in intcomma() #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/humanize/__init__.py
View file
Open in desktop
@@ -1,7 +1,7 @@
"""Main package for humanize."""

from humanize.filesize import naturalsize
from humanize.i18n import activate, deactivate, thousands_separator
from humanize.i18n import activate, deactivate, decimal_separator, thousands_separator
from humanize.number import (
apnumber,
clamp,
Expand Down Expand Up @@ -36,6 +36,7 @@
"apnumber",
"clamp",
"deactivate",
"decimal_separator",
"fractional",
"intcomma",
"intword",
Expand Down
20 changes: 19 additions & 1 deletion src/humanize/i18n.py
View file
Open in desktop
Expand Up @@ -5,7 +5,7 @@
import os.path
from threading import local

__all__ = ["activate", "deactivate", "thousands_separator"]
__all__ = ["activate", "deactivate", "decimal_separator", "thousands_separator"]

_TRANSLATIONS: dict[str | None, gettext_module.NullTranslations] = {
None: gettext_module.NullTranslations()
Expand All @@ -19,6 +19,11 @@
"fr_FR": " ",
}

# Mapping of locale to decimal separator
_DECIMAL_SEPARATOR = {
"de_DE": ",",
}


def _get_default_locale_path() -> str | None:
try:
Expand Down Expand Up @@ -173,3 +178,16 @@ def thousands_separator() -> str:
except (AttributeError, KeyError):
sep = ","
return sep


def decimal_separator() -> str:
"""Return the decimal separator for a locale, default to dot.

Returns:
str: Decimal separator.
"""
try:
sep = _DECIMAL_SEPARATOR[_CURRENT.locale]
except (AttributeError, KeyError):
sep = "."
return sep
10 changes: 6 additions & 4 deletions src/humanize/number.py
View file
Open in desktop
Expand Up @@ -13,7 +13,7 @@
from .i18n import _ngettext
from .i18n import _ngettext_noop as NS_
from .i18n import _pgettext as P_
from .i18n import thousands_separator
from .i18n import decimal_separator, thousands_separator

if TYPE_CHECKING:
if sys.version_info >= (3, 10):
Expand Down Expand Up @@ -130,10 +130,11 @@ def intcomma(value: NumberOrString, ndigits: int | None = None) -> str:
Returns:
str: String containing commas every three digits.
"""
sep = thousands_separator()
thousands_sep = thousands_separator()
decimal_sep = decimal_separator()
try:
if isinstance(value, str):
value = value.replace(sep, "")
value = value.replace(thousands_sep, "").replace(decimal_sep, ".")
if "." in value:
value = float(value)
else:
Expand All @@ -147,8 +148,9 @@ def intcomma(value: NumberOrString, ndigits: int | None = None) -> str:
orig = "{0:.{1}f}".format(value, ndigits)
else:
orig = str(value)
orig = orig.replace(".", decimal_sep)
while True:
new = re.sub(r"^(-?\d+)(\d{3})", rf"\g<1>{sep}\g<2>", orig)
new = re.sub(r"^(-?\d+)(\d{3})", rf"\g<1>{thousands_sep}\g<2>", orig)
if orig == new:
return new
orig = new
Expand Down
6 changes: 6 additions & 0 deletions tests/test_i18n.py
View file
Open in desktop
Expand Up @@ -44,6 +44,12 @@ def test_intcomma() -> None:
try:
humanize.i18n.activate("de_DE")
assert humanize.intcomma(number) == "10.000.000"
assert humanize.intcomma(1_234_567.8901) == "1.234.567,8901"
assert humanize.intcomma(1_234_567.89) == "1.234.567,89"
assert humanize.intcomma("1234567,89") == "1.234.567,89"
assert humanize.intcomma("1.234.567,89") == "1.234.567,89"
assert humanize.intcomma("1.234.567,8") == "1.234.567,8"

humanize.i18n.activate("fr_FR")
assert humanize.intcomma(number) == "10 000 000"
Comment on lines 44 to 54
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

humanize.i18n.activate("EN")
assert humanize.intcomma(1_234_567.8901) == "1,234,567.8901"

It would be good to add a test for a non-German locale to show that the original behavior is preserved. @hugovk, are the doctests verified by the CI?

Also, it should be documented that if the input is a string, it must already match the locale. i.e. This function is not to be used to convert a string from one locale to another.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to add a test for a non-German locale

There are already tests for the default locale. Or do you mean a non-German, non-default locale?

Also, it should be documented that if the input is a string, it must already match the locale.

This was already the case with the thousands separator btw. Where would be a good place to document this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are already tests for the default locale. Or do you mean a non-German, non-default locale?

This is exactly it. I didn't realize localization tests were separate from default locale tests. Thanks!

Where would be a good place to document this?

In the intcomma docstring would be the best place. It may already be the case, but it is not documented as such.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how best to write this, given that it doesn't really even mention that strings are allowed, except in two examples and partially in the arguments.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to add a test for a non-German locale to show that the original behavior is preserved. @hugovk, are the doctests verified by the CI?

Yes, doctests are run by pytest and pytest is run on the CI.


Expand Down