Clamp day-of-month when a daterange step overflows into a shorter month - #507
Open
afonsojanu wants to merge 1 commit into
Open
afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
daterange()'s month/year stepping kept the anchor day fixed and passed it straight to date.replace(), so any step that landed on a day past the end of the target month (day 29 to 31 into February, or 31 into a 30-day month) raised ValueError instead of yielding. A monthly walk anchored on a month-end date is a normal thing to build, so this is a crash on ordinary input rather than an edge case. Clamp the day to the last valid day of the target month instead, following the same convention dateutil's relativedelta uses. Added a test covering the leap/non-leap February case and stepping backwards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The change
daterange()with a month/yearstepcrashes whenever advancing lands theanchor day on a day that doesn't exist in the target month, e.g.:
_advance()carried the day over unchanged intodate.replace(), so Jan 31advancing by one month tried to build Feb 31 and blew up. A monthly sequence
anchored on a month-end date is ordinary input, not an edge case, so this
fixes it by clamping the day to the last valid day of the target month
(the same convention
dateutil.relativedeltauses) instead of raising.Closes #500.
The backstory
Went looking through the open
timeutilsissues for something concrete tofix. #500 already had a full repro, root cause, and a menu of possible
fixes from the reporter, so I reproduced the crash locally first rather than
taking that at face value, then picked the clamp-to-last-day option since
it never raises and matches existing precedent elsewhere. Added a test that
covers both the leap and non-leap February case plus stepping backwards.
AI assistance
daterangecrashing on month-step day overflow) locally, confirm it's real, implement a fix, and add tests before opening the PR.