Skip to content

Fix dehumanize failing on singular time unit nouns#1254

Open
rstar327 wants to merge 1 commit intoarrow-py:masterfrom
rstar327:fix-dehumanize-singular
Open

Fix dehumanize failing on singular time unit nouns#1254
rstar327 wants to merge 1 commit intoarrow-py:masterfrom
rstar327:fix-dehumanize-singular

Conversation

@rstar327
Copy link

@rstar327 rstar327 commented Feb 26, 2026

Pull Request Checklist

  • 🧪 Added tests for changed code.
  • 🛠️ All tests pass when run locally (run tox or make test to find out!).
  • 🧹 All linting checks pass when run locally (run tox -e lint or make lint to find out!).
  • 📚 Updated documentation for changed code.
  • ⏩ Code is up-to-date with the master branch.

Description of Changes

dehumanize("1 day ago") raised ValueError while dehumanize("1 days ago") worked correctly. Same issue affected all singular forms: "1 hour ago", "1 minute ago", "1 week ago", etc.

The fix makes the trailing 's' optional in the regex pattern so both singular and plural forms match.

Added test_singular_units covering all 7 time units in both past and future forms. All 1905 tests pass (1904 existing + 1 new).

Closes: #1150

@codecov
Copy link

codecov bot commented Feb 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (b423717) to head (3c7ab29).

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1254   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines         2315      2317    +2     
  Branches       358       359    +1     
=========================================
+ Hits          2315      2317    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@krisfremen krisfremen left a comment

Choose a reason for hiding this comment

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

This feels very biased towards english and/or locales that use s for their plurals. Merging something like this would just invite more hacky patches to add onto the other locales.

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.

The dehumanize method doesn't recognize singular nouns

2 participants