Conversation
When both decades and years are present (e.g. '-1 decade +2 years'),
the code in get_kwargs() unconditionally overwrote explicit_signs['years']
with the decades sign value:
if 'decades' in explicit_signs:
explicit_signs['years'] = explicit_signs['decades']
This destroyed the user-supplied +/- that was recorded for years during
the PATTERN match loop, causing _parse_date() to mishandle the sign of
the merged years value.
Fix: only propagate the decades sign to years when years did not carry
its own explicit sign (explicit_signs.get('years') is False/absent).
Regression tests added for:
- '-1 decade' (only decades, negative)
- '-1 decade 2 years' (decades negative, years implicit)
- '-1 decade +2 years' (decades negative, years explicitly positive)
- '+2 years' (only years, explicitly positive future)
- '1 decade +2 years' (decades implicit, years explicitly positive)
- '+1 decade -2 years' (decades explicitly positive, years explicitly negative)
Fixes #1304
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1330 +/- ##
=======================================
Coverage 97.10% 97.10%
=======================================
Files 235 235
Lines 2904 2904
=======================================
Hits 2820 2820
Misses 84 84 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes a sign-handling bug in the freshness/relative-date parser where combining decades and years could overwrite an explicitly provided sign on years (e.g. -1 decade +2 years), leading to incorrect direction when computing the final relative delta.
Changes:
- Update
FreshnessDateDataParser.get_kwargs()to only propagate thedecadesexplicit-sign flag toyearswhenyearsdoes not already have its own explicit sign. - Add regression tests covering mixed-sign and explicit-sign combinations for decades+years (and standalone explicitly-signed years).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
dateparser/freshness_date_parser.py |
Preserves an explicit +/- on years when merging decades into years, preventing incorrect sign inversion. |
tests/test_freshness_date_parser.py |
Adds regression coverage for the decades+years explicit-sign interaction, including mixed-sign cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| param("100 decades", ago={"years": 1000}, period="year"), | ||
| # Regression tests for #1304: explicit signs preserved with decades + years | ||
| param("-1 decade", ago={"years": 10}, period="year"), | ||
| param("-1 decade 2 years", ago={"years": 8}, period="year"), |
There was a problem hiding this comment.
Shouldn’t the expectation be 12 here?
When both decades and years are present (e.g. '-1 decade +2 years'), the code in get_kwargs() unconditionally overwrote explicit_signs['years'] with the decades sign value:
This destroyed the user-supplied +/- that was recorded for years during the PATTERN match loop, causing _parse_date() to mishandle the sign of the merged years value.
Fix: only propagate the decades sign to years when years did not carry its own explicit sign (explicit_signs.get('years') is False/absent).
Regression tests added for:
Fixes #1304