Support decimal values in dehumanize - #1334
Conversation
`dehumanize("2 days 3.5 hours ago")` shifted by 2 days and 5 hours: the
number pattern was `\d+`, so it matched the digits after the separator
and ignored the ones before it.
Match an optional decimal fraction and keep the value as a float when
one is present. Integers still parse as int, so nothing changes for the
strings humanize() produces.
Both `.` and `,` are accepted as the separator: dehumanize input is
written by hand, and the locale objects carry no separator information.
A separator only counts when digits follow it, so this cannot fire on
a locale's punctuation.
Fractional months and years still raise, from relativedelta, which
cannot represent them unambiguously.
Fixes arrow-py#1237
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1334 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 2315 2317 +2
Branches 358 358
=========================================
+ Hits 2315 2317 +2 ☔ View full report in Codecov by Harness. |
There was a problem hiding this comment.
Pull request overview
This PR fixes Arrow.dehumanize() so it correctly parses fractional (decimal) quantities like "3.5 hours" (and "3,5 hours") instead of incorrectly matching only the digits after the separator, addressing issue #1237.
Changes:
- Introduces a shared
_NUMBER_PATTERNthat matches integers or decimals (\d+(?:[.,]\d+)?) and uses it both for timeframe matching and number extraction. - Parses matched numeric values as
intwhen integral andfloatwhen fractional (normalizing,to.), relying onrelativedeltafloat support for seconds–weeks. - Adds tests covering fractional units (including multiple units and comma separator) and pins the
ValueErrorbehavior for fractional months/years.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
arrow/arrow.py |
Updates dehumanize() regex matching and numeric parsing to support decimal values via a shared _NUMBER_PATTERN. |
tests/test_arrow.py |
Adds targeted tests for decimal parsing, comma separators, multi-unit strings, and fractional months/years error behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A decimal value makes change_value a float, but its type was inferred from the int-only branch above and time_object_info was built with dict.fromkeys(..., 0), so mypy rejected both assignments. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The red A truncated PyPI download while building pyyaml from source on the pypy runner. Every other job in the matrix is green, and #1333 failed identically on the same runner in a separate run. A re-run should clear it — I do not have permission to trigger one here. Re-ran the suite locally on this branch to be sure: 1903 passed, 1 skipped. |
Fixes #1237.
Problem
The number pattern is
\d+, so for3.5 hoursthe search lands on the digits after the separator and the3is dropped. Expected<Arrow [2025-12-08T05:30:00+00:00]>.Fix
The pattern becomes
\d+(?:[.,]\d+)?, shared as_NUMBER_PATTERNbetween the two places that needed it (the per-timeframe search string and the number extraction). A matched value keeps parsing asintwhen it has no fraction, so nothing changes for the stringshumanize()produces — only the new decimal case yields afloat.relativedeltaaccepts floats for seconds through weeks, soshift()needed no change.On the separator
Both
.and,are accepted, as the issue suggests. The locale objects carry no decimal-separator information, anddehumanizeinput is written by hand rather than produced byhumanize(), so there is nothing to key the choice off. The separator only counts when digits follow it, which is what keeps it from firing on a locale's own punctuation.The trade-off is that digit grouping (
"1,500 hours") is read as1.5. That input is already wrong today —\d+matches just1— so this is not a regression, but it is not a case I tried to support; I noted it in a comment next to the pattern. Happy to restrict to.only if you would rather not accept the ambiguity.Fractional months and years
relativedeltarejects these outright ("Non-integer years and months are ambiguous and not currently supported"), which surfaces as aValueErrorfromdehumanize. That seemed better than silently truncating; there is a test pinning it.Tests
test_fractional_value— hours, minutes and days, both directions.test_fractional_value_comma_separator—1,5matches1.5.test_fractional_value_with_multiple_units— the exact case from the issue, plus its future form.test_fractional_months_and_years_are_rejected.Full suite passes (1903 passed, 1 skipped).
🤖 Generated with Claude Code