Skip to content

Fix KeyError in diff_for_humans for the zh locale - #1001

Open
dylanpulver wants to merge 1 commit into
python-pendulum:masterfrom
dylanpulver:fix/zh-diff-for-humans-placeholder
Open

Fix KeyError in diff_for_humans for the zh locale#1001
dylanpulver wants to merge 1 commit into
python-pendulum:masterfrom
dylanpulver:fix/zh-diff-for-humans-placeholder

Conversation

@dylanpulver

Copy link
Copy Markdown

Pull Request Check List

  • Added tests for changed code.
  • Updated documentation for changed code.

diff_for_humans() raises KeyError: 'time' for the zh locale whenever it is given a
second datetime (the non-relative-to-now form), for every unit:

>>> a = pendulum.datetime(2024, 6, 15, 12, 0, 5)
>>> b = pendulum.datetime(2024, 6, 15, 12, 0, 0)
>>> a.diff_for_humans(b, locale="zh")
KeyError: 'time'
>>> a.diff_for_humans(b, locale="ja")
'数秒 後'

zh/custom.py:9-10 holds "after": "{time}后" / "before": "{time}前" — named placeholders —
while DifferenceFormatter.format() substitutes positionally
(difference_formatter.py:115 and :159, both locale.get(key).format(time)). All 28 other
locales use {0}, so this changes only the zh data; no code change.

Base is master per the template note (bug fix, not a behaviour change).

Why nothing caught it: the relative-to-now form reads translations.relative.* (the
generated CLDR table) and never touches these two custom entries, and there is no
tests/localization/test_zh.pyzh, en* and pt_br are the locales with no test file.
Added one for zh, modelled on test_ja.py, covering both forms.

How it was found, since it's a reasonable question: a sweep, not a bug report from real
use. I called diff_for_humans across all 29 locales x 8 durations x both signs x
absolute on/off (928 calls) and looked for exceptions or unsubstituted {. zh was the
only locale that failed, 16/32 of its calls — exactly the is_now=False, absolute=False path.
{time} appears nowhere else in src/ or tests/.

Measured: pytest tests 1840 passed / 3 skipped before, 1841 / 3 after (same command,
same venv). Mutants: reverting the two strings and keeping the new test re-raises all 16
sweep failures and fails test_zh.py.

Honest limitation: a second mutant — keeping {time} and making the formatter call
.format(time, time=time) — also passes the sweep and the new test. So the test does not by
itself pick between the two remedies. I chose the data fix because the alternative sanctions
two placeholder conventions in the locale files; if you would rather the formatter be
tolerant, say so and I'll switch it.

Not tested: I did not add test files for en, en_gb, en_us or pt_br, which also
have none. ruff format --check reports a pre-existing blank-line diff in 28 of the 29
locales/*/custom.py files including this one; I left it alone.

Disclosure: the diff, the test and this description were drafted with Claude Opus 4.8.

zh/custom.py used a NAMED placeholder in "after"/"before" ("{time}后",
"{time}前") while DifferenceFormatter.format() substitutes positionally
(difference_formatter.py:115,159 call locale.get(key).format(time)).
So the two-datetime form raises KeyError: 'time' for every unit:

    a.diff_for_humans(b, locale="zh")   ->  KeyError: 'time'

All 28 other locales use "{0}". zh is switched to match; no code change.
The relative-to-now form was unaffected because it reads the CLDR
translations table rather than these custom entries, and there was no
tests/localization/test_zh.py at all, so nothing exercised this path.

Adds that test file, covering the relative-to-now form and the
two-datetime form that was crashing.
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.

1 participant