Skip to content

Honor the locale decimal separator in naturalsize() - #463

Open
ekanshul wants to merge 1 commit into
python-humanize:mainfrom
ekanshul:fix-460-naturalsize-locale-decimal
Open

ekanshul wants to merge 1 commit into
python-humanize:mainfrom
ekanshul:fix-460-naturalsize-locale-decimal

Conversation

@ekanshul

Copy link
Copy Markdown

Fixes #460

Changes proposed in this pull request:

  • Use decimal_separator() in naturalsize() so locales with a comma radix (fr_FR, de_DE, it_IT, …) render 1,5 Mo instead of 1.5 Mo
  • Skip the rewrite when the separator is "." so the default English path stays a single format + concat
  • Add a filesize regression test and update the existing French i18n expectations

intcomma() and intword() already honor decimal_separator(). #378 left naturalsize as the remaining loose end.

intcomma() and intword() already swap "." for decimal_separator().
naturalsize() left the mantissa dotted, so fr_FR / de_DE / it_IT
showed "1.5 Mo" instead of "1,5 Mo". Rewrite only when the locale
separator is not ".", so the default English path stays a single
format + concat.

Fixes python-humanize#460

@Muguett-DBY Muguett-DBY left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I reproduced a cache leak in the new regression and suggested a small cleanup below.

Checked exact head b0f07f0 on Windows / Python 3.12.14 with selected test bodies and a small test catalog. Codex-assisted; I didn't run pytest/tox or compile .mo files.

Comment thread tests/test_filesize.py
Comment on lines +108 to +121
def test_naturalsize_uses_locale_decimal_separator() -> None:
"""Locales with a comma radix should not keep a dotted mantissa.

Uses a NullTranslations catalog so this does not depend on compiled .mo files.
"""
humanize.i18n._TRANSLATIONS["fr_FR"] = gettext.NullTranslations()
humanize.i18n._CURRENT.locale = "fr_FR"
try:
assert humanize.naturalsize(1_500_000) == "1,5 MB"
assert humanize.naturalsize(3000, gnu=True) == "2,9K"
assert humanize.naturalsize(3000, binary=True) == "2,9 KiB"
finally:
humanize.i18n.deactivate()
assert humanize.naturalsize(1_500_000) == "1.5 MB"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please scope this cache entry with the monkeypatch fixture. deactivate() only resets the current locale; it leaves the NullTranslations object in _TRANSLATIONS["fr_FR"]. A later activate("fr_FR") then reuses the dummy catalog instead of loading or retaining the French translations.

Running this test body before the existing French naturalsize(42) case gives 42 Bytes instead of 42 octets. I reproduced this with the French cache entry absent and with a previously cached French catalog. monkeypatch.setitem restores the previous entry, or removes the inserted key, at teardown; test_update_translations already uses that pattern.

Suggested change
def test_naturalsize_uses_locale_decimal_separator() -> None:
"""Locales with a comma radix should not keep a dotted mantissa.
Uses a NullTranslations catalog so this does not depend on compiled .mo files.
"""
humanize.i18n._TRANSLATIONS["fr_FR"] = gettext.NullTranslations()
humanize.i18n._CURRENT.locale = "fr_FR"
try:
assert humanize.naturalsize(1_500_000) == "1,5 MB"
assert humanize.naturalsize(3000, gnu=True) == "2,9K"
assert humanize.naturalsize(3000, binary=True) == "2,9 KiB"
finally:
humanize.i18n.deactivate()
assert humanize.naturalsize(1_500_000) == "1.5 MB"
def test_naturalsize_uses_locale_decimal_separator(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Locales with a comma radix should not keep a dotted mantissa.
Uses a NullTranslations catalog so this does not depend on compiled .mo files.
"""
monkeypatch.setitem(
humanize.i18n._TRANSLATIONS, "fr_FR", gettext.NullTranslations()
)
humanize.i18n._CURRENT.locale = "fr_FR"
try:
assert humanize.naturalsize(1_500_000) == "1,5 MB"
assert humanize.naturalsize(3000, gnu=True) == "2,9K"
assert humanize.naturalsize(3000, binary=True) == "2,9 KiB"
finally:
humanize.i18n.deactivate()
assert humanize.naturalsize(1_500_000) == "1.5 MB"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

naturalsize() ignores the locale decimal separator

2 participants