Skip to content

Preserve invalid string inputs in intcomma - #453

Open
fading723 wants to merge 1 commit into
python-humanize:mainfrom
fading723:fix/intcomma-invalid-string
Open

fading723 wants to merge 1 commit into
python-humanize:mainfrom
fading723:fix/intcomma-invalid-string

Conversation

@fading723

Copy link
Copy Markdown

intcomma("hello, world") currently returns "hello world". Separator normalization overwrites the input before parsing, so the fallback returns modified text when parsing fails. German and French locales can also remove punctuation or spaces and replace commas with dots.

Changes proposed in this pull request:

  • Normalize separators in a separate variable and replace the input only after numeric conversion succeeds.
  • Add regression cases for ordinary text, malformed numeric strings, and an unsupported exponent string, with default and explicit precision.
  • Cover the same fallback behavior with German and French separators.

Validation on Windows, CPython 3.12.14:

  • All 24 new regression cases fail against the original implementation and pass with the fix.
  • Full tests and source doctests: 893 passed, 2 skipped. The skipped translation-update tests require Bash/gettext; all 36 locale catalogs were compiled locally with polib.
  • Ruff, Black, mypy, all applicable configured prek hooks, and git diff --check pass.

Benchmark tests ran with timing disabled.

@Wkhanex Wkhanex 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.

25 English-locale checks passed for malformed/grouped/exponent strings with default ndigits, ndigits=0 and ndigits=2, plus valid integer/decimal controls. Inputs such as "12,34x", "1,2.3.4", "1,000 dollars" and "1,234e2" now return the original string byte-for-byte on conversion failure. I used Python 3.12.14 against commit fc76fd3.

The exponent case is worth a regression: normalized text can be float-readable while still failing the later int conversion, so preserving the original value until conversion completes matters in that branch too. Valid "1,234.5" still formats as "1,234.5". I ran standalone assertions against fetched PR source; only the missing build-generated version metadata was stubbed. I did not run pytest or translation-catalog tests.

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.

2 participants