Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: DatetimeIndex.is_year_start breaks on custom business days frequencies bigger then 1C #58665

Conversation

natmokval
Copy link
Contributor

@natmokval natmokval commented May 10, 2024

@natmokval natmokval marked this pull request as ready for review May 10, 2024 11:59
@natmokval natmokval added Bug Frequency DateOffsets labels May 10, 2024
@@ -419,6 +419,7 @@ Interval
Indexing
^^^^^^^^
- Bug in :meth:`DataFrame.__getitem__` returning modified columns when called with ``slice`` in Python 3.12 (:issue:`57500`)
- Bug in :meth:`DatetimeIndex.is_year_start` and :meth:`DatetimeIndex.is_quarter_start` does not raise on Custom business days frequencies bigger then "1C" (:issue:`58664`)
Copy link
Member

Choose a reason for hiding this comment

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

Could you put this under Datetimelike?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, updated


# YearBegin(), BYearBegin() use month = starting month of year.
# QuarterBegin(), BQuarterBegin() use startingMonth = starting
# month of year. Other offsets use month, startingMonth as ending
# month of year.

if (freqstr[0:2] in ["MS", "QS", "YS"]) or (
freqstr[1:3] in ["MS", "QS", "YS"]):
if freq_name.lstrip("B")[0:2] in ["MS", "QS", "YS"]:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if freq_name.lstrip("B")[0:2] in ["MS", "QS", "YS"]:
if freq_name.lstrip("B")[0:2] in ["MS", "QS", "YS"]:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@MarcoGorelli
Copy link
Member

MarcoGorelli commented May 10, 2024

this needs rebasing, as it contains commits from the other PR (and could you move the whatsnew from that one to DatetimeLike please?)

(other than that, it looks like this identifies + fixes the issue, nice)

@natmokval
Copy link
Contributor Author

natmokval commented May 11, 2024

this needs rebasing, as it contains commits from the other PR (and could you move the whatsnew from that one to DatetimeLike please?)

thanks, rebased and moved the whatsnew note from Indexing to DatetimeLike

@mroeschke mroeschke added this to the 3.0 milestone May 11, 2024
@mroeschke mroeschke merged commit 34177d6 into pandas-dev:main May 11, 2024
47 checks passed
@mroeschke
Copy link
Member

Thanks @natmokval

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Frequency DateOffsets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: DatetimeIndex.is_year_start breaks on custom business days frequencies bigger then 1C
3 participants