Fix explicit +/- signs in relative date offsets #1303
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1303 +/- ##
==========================================
+ Coverage 96.60% 96.62% +0.01%
==========================================
Files 235 235
Lines 2889 2905 +16
==========================================
+ Hits 2791 2807 +16
Misses 98 98 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes parsing of relative date expressions containing explicit +/- time offsets (e.g., "yesterday +1h"), addressing issue #1299.
Changes:
- Updated
FreshnessDateDataParserto recognize signed numeric quantities (e.g.,+1 hour,-30 minutes) and apply them correctly. - Added regression tests validating
"yesterday"/"tomorrow" +/- <offset>parsing behavior viadateparser.parse().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
dateparser/freshness_date_parser.py |
Extends unit parsing regex to accept +/- and adjusts relativedelta application to respect explicit signs. |
tests/test_date_parser.py |
Adds parameterized coverage for "yesterday"/"tomorrow" +/- offsets plus targeted regression assertions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if "decades" in kwargs: | ||
| kwargs["years"] = 10 * kwargs["decades"] + kwargs.get("years", 0) | ||
| if "decades" in explicit_signs: |
There was a problem hiding this comment.
When both decades and years are present in the input (e.g., "-1 decade +2 years"), this logic overwrites explicit_signs["years"] with the decades flag, potentially losing whether years had an explicit sign. Consider only propagating the decades sign to years when years wasn't explicitly present, or combining them in a way that preserves an explicit sign on years if provided.
| if "decades" in explicit_signs: | |
| if "decades" in explicit_signs and "years" not in explicit_signs: |
| def test_yesterday_plus_and_minus_not_equal(self): | ||
| """Verify plus and minus time offsets produce different results.""" | ||
| base_date = datetime(2026, 1, 19, 12, 0, 0) | ||
|
|
||
| plus = parse( | ||
| "yesterday +1h", | ||
| settings={"RELATIVE_BASE": base_date, "RETURN_AS_TIMEZONE_AWARE": False}, | ||
| ) | ||
| minus = parse( | ||
| "yesterday -1h", | ||
| settings={"RELATIVE_BASE": base_date, "RETURN_AS_TIMEZONE_AWARE": False}, | ||
| ) | ||
|
|
||
| self.assertIsNotNone(plus) | ||
| self.assertIsNotNone(minus) | ||
| self.assertNotEqual(plus, minus) | ||
|
|
There was a problem hiding this comment.
test_yesterday_plus_and_minus_not_equal is redundant with test_relative_date_with_time_offset, which already asserts the exact expected timestamps for both plus and minus offsets (implying they differ). Consider removing this test or folding the not-equal assertion into the parameterized cases to reduce duplicate coverage and runtime.
| def test_yesterday_plus_and_minus_not_equal(self): | |
| """Verify plus and minus time offsets produce different results.""" | |
| base_date = datetime(2026, 1, 19, 12, 0, 0) | |
| plus = parse( | |
| "yesterday +1h", | |
| settings={"RELATIVE_BASE": base_date, "RETURN_AS_TIMEZONE_AWARE": False}, | |
| ) | |
| minus = parse( | |
| "yesterday -1h", | |
| settings={"RELATIVE_BASE": base_date, "RETURN_AS_TIMEZONE_AWARE": False}, | |
| ) | |
| self.assertIsNotNone(plus) | |
| self.assertIsNotNone(minus) | |
| self.assertNotEqual(plus, minus) |
Close #1299