BUG: Raise error if not busdaycalendar #61250
Conversation
| @@ -78,7 +80,7 @@ def test_weekmask_and_holidays(self): | |||
|
|
|||
| @pytest.mark.filterwarnings("ignore:Non:pandas.errors.PerformanceWarning") | |||
| def test_calendar(self): | |||
| calendar = USFederalHolidayCalendar() | |||
There was a problem hiding this comment.
So was this just getting ignored before? If not and the behavior was working as expected, we should continue supporting these arguments
There was a problem hiding this comment.
From my understanding, rhshadrach thinks we should only support np.busdaycalender for the argument calendar. He said in the issue, "I think we should raise when a calendar that is not an np.busdaycalender is passed instead of silently ignoring it.".
Therefore, I have edited the constructor to raise a TypeError if anything other than None or a np.busdaycalender is passed. Happy to change if needed!
There was a problem hiding this comment.
Right, I agree in principle, but with the modified docs and tests in this PR we have some testing with AbstractHolidayCalendar and it's subclasses, so this PR would introduce breaking behavior.
Therefore, I would suggest first investigating if those docs/tests are producing correct behavior. If so, I would suggest calling the original issue a bug that needs fixing, but it would still be good to add validation to allow calendar: None | np.businesscalendar | AbstractHolidayCalendar
Closes #60647. Raises TypeError if anything other than
Noneornp.busdaycalendaris passed tocalendar. Also added test for this exception as well as a note in whatsnewdoc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.