Skip to content

fix: Preserve explicit signs for years when combined with decades#1330

Open
serhii73 wants to merge 1 commit into
masterfrom
fix/1304
Open

fix: Preserve explicit signs for years when combined with decades#1330
serhii73 wants to merge 1 commit into
masterfrom
fix/1304

Conversation

@serhii73
Copy link
Copy Markdown
Collaborator

@serhii73 serhii73 commented May 7, 2026

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

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
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.10%. Comparing base (373ede9) to head (d06a1d5).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 the decades explicit-sign flag to years when years does 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.

@serhii73 serhii73 requested review from AdrianAtZyte and wRAR May 7, 2026 10:23
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"),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn’t the expectation be 12 here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I also wasn't sure about this.

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.

Explicit signs lost when combining decades and years in freshness date parser

4 participants