Skip to content

Reject non-positive date_range steps#1339

Open
CodingFeng101 wants to merge 3 commits into
scrapinghub:masterfrom
CodingFeng101:codex/reject-nonpositive-date-range-step
Open

Reject non-positive date_range steps#1339
CodingFeng101 wants to merge 3 commits into
scrapinghub:masterfrom
CodingFeng101:codex/reject-nonpositive-date-range-step

Conversation

@CodingFeng101

Copy link
Copy Markdown

Summary

Reject date_range() steps that do not advance the generated date. Passing values such as months=0 or days=-1 currently allows the generator to yield the start date and then continue without making forward progress, which can hang callers that consume the full iterator with list(...).

Changes

  • Add a positive-step validation in dateparser.date.date_range().
  • Add a focused regression test for zero-month and negative-day steps.

Validation

  • python -m pytest tests/test_date_range.py -q -> 2 passed
  • Manual smoke check confirmed normal daily and monthly ranges still produce the same forward dates.

@AdrianAtZyte

Copy link
Copy Markdown
Contributor

Does Python’s range implement any kind of validation like this, or does it leave it up to the user to pass the right value?

@CodingFeng101

Copy link
Copy Markdown
Author

Thanks for checking. Python's range() validates the one case that cannot make progress: range(..., step=0) raises ValueError: range() arg 3 must not be zero. It allows negative steps because range() has explicit backward-iteration semantics when the caller provides matching start/stop values.

My thinking here was that date_range() currently advances from begin toward end, and non-positive deltas such as months=0 or days=-1 can make the generator fail to progress toward that end in common calls like list(date_range(begin, end, months=0)). So the validation is meant to fail early for non-progressing steps instead of leaving callers with an accidental infinite iterator.

That said, if you prefer to leave this to callers or want an issue first before changing this behavior, I understand and am happy to close the PR.

Comment thread tests/test_date_range.py Outdated

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.

Could you move this to the existing TestDateRangeFunction?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Moved it into the existing TestDateRangeFunction in 4cbea3a and removed the separate test file.

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.11%. Comparing base (08c78d3) to head (4cbea3a).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1339   +/-   ##
=======================================
  Coverage   97.11%   97.11%           
=======================================
  Files         235      235           
  Lines        2909     2911    +2     
=======================================
+ Hits         2825     2827    +2     
  Misses         84       84           

☔ View full report in Codecov by Harness.
📢 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.

@AdrianAtZyte

Copy link
Copy Markdown
Contributor

Please, run pre-commit run --all-files.

@CodingFeng101

Copy link
Copy Markdown
Author

Ran pre-commit run --all-files. ruff-format reformatted the new date range test in 1150f2f, and a second run now passes:

ruff.....................................................................Passed
ruff-format..............................................................Passed

I also ran git diff --check, which passed. I tried rerunning python -m pytest tests/test_date.py::TestDateRangeFunction -q locally, but on Windows collection fails before reaching the test because tests/test_date.py imports time.tzset, which is not available on Windows.

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.

2 participants