Skip to content

BUG: Return type discrepancy in USFederalHolidayCalendar #49118

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

Merged
merged 43 commits into from
Nov 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
8281084
GH49075 looks weird. Let's add a test and go from there.
roadswitcher Oct 15, 2022
8d1b868
Some cleanup.
roadswitcher Oct 15, 2022
eb5c8a8
First pass at the fix.
roadswitcher Oct 15, 2022
b00c83f
Missed a space, failed formatting check.
roadswitcher Oct 15, 2022
1720600
pre-commit would have caught that, eh.
roadswitcher Oct 15, 2022
d87891f
Missed a spot.
roadswitcher Oct 15, 2022
ad0a017
Update test_federal.py
roadswitcher Oct 15, 2022
d8e9f9d
Merge branch 'pandas-dev:main' into odd_calendar_behavior
roadswitcher Oct 16, 2022
c07f068
Well, that was subtle.
roadswitcher Oct 16, 2022
b423bb7
Changed the test.
roadswitcher Oct 16, 2022
9f89326
Git mistake.
roadswitcher Oct 16, 2022
992143c
Backing away from keyboard now.
roadswitcher Oct 16, 2022
8e516f3
Merge branch 'pandas-dev:main' into odd_calendar_behavior
roadswitcher Oct 29, 2022
7486888
next: write more tests.
roadswitcher Oct 29, 2022
d63ea70
precommit local run.
roadswitcher Oct 29, 2022
e40ce4d
User specified half-open date intervals can return inconsistent results.
roadswitcher Oct 29, 2022
c953651
Added logic to close open time intervals into AbstractHolidayCalender
roadswitcher Oct 30, 2022
68f8db4
OK, I've learned pre-commit doesn't pass wildcards all the way down a…
roadswitcher Oct 30, 2022
344238e
Merge branch 'pandas-dev:main' into odd_calendar_behavior
roadswitcher Oct 30, 2022
3172392
Ensure DatetimeIndex returned in _apply_rule
roadswitcher Oct 30, 2022
824abb6
Merge branch 'odd_calendar_behavior' of github.com:roadswitcher/panda…
roadswitcher Oct 30, 2022
cb6ab53
Ensure DatetimeIndex returned in _apply_rule
roadswitcher Oct 30, 2022
b865481
Caught formatting.
roadswitcher Oct 30, 2022
0c8e325
Missed running isort locally.
roadswitcher Oct 30, 2022
718daa0
Merge branch 'main' into odd_calendar_behavior
roadswitcher Oct 30, 2022
5aca82d
Well, that was subtle.
roadswitcher Nov 1, 2022
c6b2d5b
Changed URL to current OPM source-of-record, updated whatsnew
roadswitcher Nov 1, 2022
da6c1a3
Merge branch 'main' into odd_calendar_behavior
roadswitcher Nov 1, 2022
a8a6244
Update v2.0.0.rst
roadswitcher Nov 1, 2022
ec087a3
Add dtype to empty index ( caught by mypy in CI, not part of pre-comm…
roadswitcher Nov 1, 2022
a6242a9
Updated holiday.py
roadswitcher Nov 1, 2022
8922123
... and forgot to pre-commit run black
roadswitcher Nov 1, 2022
592f705
Merge branch 'main' into odd_calendar_behavior
roadswitcher Nov 1, 2022
cd848c1
Merge branch 'main' into odd_calendar_behavior
roadswitcher Nov 1, 2022
a502a06
Update pandas/tseries/holiday.py
roadswitcher Nov 1, 2022
67c83b8
Update v2.0.0.rst
roadswitcher Nov 1, 2022
1ae7c11
Merge branch 'main' into odd_calendar_behavior
roadswitcher Nov 1, 2022
6188c80
Added test to test_federal.py to ensure comparison against known-good…
roadswitcher Nov 1, 2022
4283115
Add change to test_federal to compare against constructed DatetimeInd…
roadswitcher Nov 2, 2022
6122af8
Merge branch 'main' into odd_calendar_behavior
roadswitcher Nov 2, 2022
ad311f0
Merge branch 'main' into odd_calendar_behavior
roadswitcher Nov 2, 2022
176f3e3
Merge branch 'main' into odd_calendar_behavior
roadswitcher Nov 2, 2022
45521a7
Update doc/source/whatsnew/v2.0.0.rst
mroeschke Nov 2, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v2.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ Datetimelike
- Bug in :func:`to_datetime` was raising on invalid offsets with ``errors='coerce'`` and ``infer_datetime_format=True`` (:issue:`48633`)
- Bug in :class:`DatetimeIndex` constructor failing to raise when ``tz=None`` is explicitly specified in conjunction with timezone-aware ``dtype`` or data (:issue:`48659`)
- Bug in subtracting a ``datetime`` scalar from :class:`DatetimeIndex` failing to retain the original ``freq`` attribute (:issue:`48818`)
-
- Bug in ``pandas.tseries.holiday.Holiday`` where a half-open date interval causes inconsistent return types from :meth:`USFederalHolidayCalendar.holidays` (:issue:`49075`)

Timedelta
^^^^^^^^^
Expand Down
20 changes: 20 additions & 0 deletions pandas/tests/tseries/holiday/test_federal.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
from datetime import datetime

from pandas import DatetimeIndex
import pandas._testing as tm

from pandas.tseries.holiday import (
AbstractHolidayCalendar,
USFederalHolidayCalendar,
USMartinLutherKingJr,
USMemorialDay,
)
Expand Down Expand Up @@ -36,3 +40,19 @@ class MemorialDay(AbstractHolidayCalendar):
datetime(1978, 5, 29, 0, 0),
datetime(1979, 5, 28, 0, 0),
]


def test_federal_holiday_inconsistent_returntype():
# GH 49075 test case
# Instantiate two calendars to rule out _cache
cal1 = USFederalHolidayCalendar()
cal2 = USFederalHolidayCalendar()

results_2018 = cal1.holidays(start=datetime(2018, 8, 1), end=datetime(2018, 8, 31))
results_2019 = cal2.holidays(start=datetime(2019, 8, 1), end=datetime(2019, 8, 31))
expected_results = DatetimeIndex([], dtype="datetime64[ns]", freq=None)

# Check against expected results to ensure both date
# ranges generate expected results as per GH49075 submission
tm.assert_index_equal(results_2018, expected_results)
tm.assert_index_equal(results_2019, expected_results)
47 changes: 47 additions & 0 deletions pandas/tests/tseries/holiday/test_holiday.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import pytest
from pytz import utc

from pandas import DatetimeIndex
import pandas._testing as tm

from pandas.tseries.holiday import (
Expand Down Expand Up @@ -264,3 +265,49 @@ def test_both_offset_observance_raises():
offset=[DateOffset(weekday=SA(4))],
observance=next_monday,
)


def test_half_open_interval_with_observance():
# Prompted by GH 49075
# Check for holidays that have a half-open date interval where
# they have either a start_date or end_date defined along
# with a defined observance pattern to make sure that the return type
# for Holiday.dates() remains consistent before & after the year that
# marks the 'edge' of the half-open date interval.

holiday_1 = Holiday(
"Arbitrary Holiday - start 2022-03-14",
start_date=datetime(2022, 3, 14),
month=3,
day=14,
observance=next_monday,
)
holiday_2 = Holiday(
"Arbitrary Holiday 2 - end 2022-03-20",
end_date=datetime(2022, 3, 20),
month=3,
day=20,
observance=next_monday,
)

class TestHolidayCalendar(AbstractHolidayCalendar):
rules = [
USMartinLutherKingJr,
holiday_1,
holiday_2,
USLaborDay,
]

start = Timestamp("2022-08-01")
end = Timestamp("2022-08-31")
year_offset = DateOffset(years=5)
expected_results = DatetimeIndex([], dtype="datetime64[ns]", freq=None)
test_cal = TestHolidayCalendar()

date_interval_low = test_cal.holidays(start - year_offset, end - year_offset)
date_window_edge = test_cal.holidays(start, end)
date_interval_high = test_cal.holidays(start + year_offset, end + year_offset)

tm.assert_index_equal(date_interval_low, expected_results)
tm.assert_index_equal(date_window_edge, expected_results)
tm.assert_index_equal(date_interval_high, expected_results)
6 changes: 4 additions & 2 deletions pandas/tseries/holiday.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,9 @@ def _apply_rule(self, dates):
-------
Dates with rules applied
"""
if dates.empty:
return DatetimeIndex([])

if self.observance is not None:
return dates.map(lambda d: self.observance(d))

Expand Down Expand Up @@ -553,8 +556,7 @@ def merge(self, other, inplace: bool = False):
class USFederalHolidayCalendar(AbstractHolidayCalendar):
"""
US Federal Government Holiday Calendar based on rules specified by:
https://www.opm.gov/policy-data-oversight/
snow-dismissal-procedures/federal-holidays/
https://www.opm.gov/policy-data-oversight/pay-leave/federal-holidays/
"""

rules = [
Expand Down