DOC/ENH: Holiday exclusion argument #61600

sharkipelago · 2025-06-07T17:07:21Z

closes DOC/ENH: Holiday documentation is missing or hard to find for re-arranged national holidays. #54382 (Replace xxxx with the GitHub issue number)
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

mroeschke · 2025-06-09T16:57:40Z

doc/source/whatsnew/v2.3.0.rst

 - :meth:`Series.str.decode` result now has :class:`StringDtype` when ``future.infer_string`` is True (:issue:`60709`)
 - :meth:`~Series.to_hdf` and :meth:`~DataFrame.to_hdf` now round-trip with :class:`StringDtype`  (:issue:`60663`)
 - Improved ``repr`` of :class:`.NumpyExtensionArray` to account for NEP51 (:issue:`61085`)
+- The :class:`Holiday` has gained the constructor argument and field ``exclude_dates`` to exclude specific datetimes from a custom holiday calendar (:issue:`54382`)


Can you move this to v3.0.0.rst?

mroeschke · 2025-06-09T17:00:43Z

pandas/tseries/holiday.py

        start_date=None,
        end_date=None,
        days_of_week: tuple | None = None,
+        exclude_dates: list[Any] | None = None,


Suggested change

exclude_dates: list[Any] | None = None,

exclude_dates: list[Timestamp] | None = None,

Let's make the users do the Timestamp conversion before passing into this object

mroeschke · 2025-06-09T17:01:38Z

pandas/tseries/holiday.py

+        self.exclude_dates = (
+            [Timestamp(ex) for ex in exclude_dates]
+            if exclude_dates is not None
+            else exclude_dates
+        )


With the constructor argument being stricter, we can just validate the arguments are all Timestamp

mroeschke · 2025-06-09T17:03:22Z

pandas/tseries/holiday.py

+            holiday_dates = DatetimeIndex(
+                [hd for hd in holiday_dates if hd not in self.exclude_dates]
+            )


Conversion to DatetimeIndex should be done in __init__

Probably better to use .difference or releated method instead

…rgument

mroeschke · 2025-06-10T15:45:51Z

pandas/tseries/holiday.py

+from typing import (
+    TYPE_CHECKING,
+)


Suggested change

from typing import (

TYPE_CHECKING,

)

from typing import TYPE_CHECKING

mroeschke · 2025-06-10T15:46:22Z

pandas/tseries/holiday.py

        start_date=None,
        end_date=None,
        days_of_week: tuple | None = None,
+        exclude_dates: list[Timestamp] | None = None,


Actually, can we type this as exclude_dates: DatetimeIndex | None = None,?

mroeschke · 2025-06-10T15:47:00Z

pandas/tseries/holiday.py

+        assert exclude_dates is None or all(
+            type(ex) == Timestamp for ex in exclude_dates
+        )


Can you raise a ValueError if exclude_dates is not a DatetimeIndex or None?

sharkipelago · 2025-06-10T17:54:03Z

pandas/tseries/holiday.py

        )
        self.end_date = Timestamp(end_date) if end_date is not None else end_date
        self.observance = observance
        assert days_of_week is None or type(days_of_week) == tuple


Should I also switch this to throw a ValueError on failing? Or would that be a separate PR?

A separate PR would be better, thanks.

For this, should I open a new issue? Or just make another PR?

Just another PR is fine

mroeschke · 2025-06-10T17:59:04Z

pandas/tseries/holiday.py

        self.observance = observance
        assert days_of_week is None or type(days_of_week) == tuple
        self.days_of_week = days_of_week
+        if exclude_dates is not None and type(exclude_dates) != DatetimeIndex:


Suggested change

if exclude_dates is not None and type(exclude_dates) != DatetimeIndex:

if not (exclude_dates is None or isinstance(exclude_dates, DatetimeIndex):

pandas/tseries/holiday.py

sharkipelago · 2025-06-11T17:14:57Z

I'm trying to figure out what the Pyodide test failing means, but struggling a bit. @mroeschke any insight in where I should look to resolve the failed check?

Also trying to just rerun the checks, but couldn't figure out how to do that without making an empty commit

mroeschke · 2025-06-11T17:25:09Z

pandas/tests/tseries/holiday/test_holiday.py

+    assert exclude.isin(original_dates).all()
+    assert ~exclude.isin(actual_dates).all()
+    assert actual_dates.isin(original_dates).all()


Could you construct the resulting DatetimeIndex and use tm.assert_index_equal i.e.

result = queens_jubilee_uk_spring_bank_holiday.dates(start, end) expected = DatetimeIndex(...) tm.assert_index_equal(result, expected)

And below as well

I added the tm.assert_index_equal() in addition to the above 3 asserts, hopefully that is okay? I can also delete the 3 assert statements from above

Yes could you delete the 3 assert statements in this test and below? Hardcoding the expected result and using tm.assert_index.equal is a stronger assertion than these statements

mroeschke · 2025-06-11T17:25:14Z

pandas/tests/tseries/holiday/test_holiday.py

+
+    original_dates = default_uk_spring_bank_holiday.dates(start, end)
+    actual_dates = queens_jubilee_uk_spring_bank_holiday.dates(start, end)
+    print(exclude.isin(original_dates).all())


Suggested change

print(exclude.isin(original_dates).all())

mroeschke · 2025-06-11T17:25:47Z

I'm trying to figure out what the Pyodide test failing means

I think that's unrelated to your PR, so no need to worry about it

…rgument

sharkipelago · 2025-06-12T21:16:58Z

Thanks for all your help on the PR so far! I really appreciate it. Are there any other changes I should make?

mroeschke · 2025-06-13T16:11:49Z

Thanks @sharkipelago

sharkipelago added 2 commits June 7, 2025 12:46

added exclude_dates argument and test for holiday class

f054afb

added entry on whatsnew

5a809e2

sharkipelago changed the title ~~Holiday exclusion argument~~ DOC/ENH: Holiday exclusion argument Jun 8, 2025

mroeschke reviewed Jun 9, 2025

View reviewed changes

mroeschke added the Frequency DateOffsets label Jun 9, 2025

sharkipelago added 4 commits June 10, 2025 09:11

Merge remote-tracking branch 'upstream/main' into holiday-exclusion-a…

891b442

…rgument

moved pull request to v3.0.0 whatsnew

aa08735

updated exclude_dates to be [Timestamp] in constructor

f019a5c

updated Holiday.dates() to use difference method

bdbb555

mroeschke reviewed Jun 10, 2025

View reviewed changes

sharkipelago added 3 commits June 10, 2025 13:40

condensed type_checking import statement to 1 line

5bca8c7

switchted exclude_dates type from list[Timestamp] to DatetimeIndex

655ca4e

raise ValueError if exclude_dates is not DatetimeIndex

f46c394

sharkipelago commented Jun 10, 2025

View reviewed changes

mroeschke reviewed Jun 10, 2025

View reviewed changes

pandas/tseries/holiday.py Show resolved Hide resolved

sharkipelago added 2 commits June 10, 2025 19:42

isinstance to check exclude_data

27cca3d

updated tests for DatetimeIndex type change

a9a745c

mroeschke reviewed Jun 11, 2025

View reviewed changes

sharkipelago added 3 commits June 11, 2025 13:49

Merge remote-tracking branch 'upstream/main' into holiday-exclusion-a…

7710e43

…rgument

added proper result-expected assertion pattern for tests

5cfd599

removed redundant asserts

2d9e4c5

mroeschke approved these changes Jun 13, 2025

View reviewed changes

mroeschke added this to the 3.0 milestone Jun 13, 2025

mroeschke merged commit 41c63ab into pandas-dev:main Jun 13, 2025
44 checks passed

sharkipelago deleted the holiday-exclusion-argument branch June 14, 2025 11:55

sharkipelago mentioned this pull request Jun 14, 2025

DOC/ENH: Holiday days_of_week value error #61658

Merged

3 tasks

	exclude_dates: list[Any] \| None = None,
	exclude_dates: list[Timestamp] \| None = None,

	if exclude_dates is not None and type(exclude_dates) != DatetimeIndex:
	if not (exclude_dates is None or isinstance(exclude_dates, DatetimeIndex):

Uh oh!

DOC/ENH: Holiday exclusion argument #61600

DOC/ENH: Holiday exclusion argument #61600

Uh oh!

Conversation

sharkipelago commented Jun 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sharkipelago Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sharkipelago commented Jun 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mroeschke commented Jun 11, 2025

Uh oh!

sharkipelago commented Jun 12, 2025

Uh oh!

Uh oh!

mroeschke commented Jun 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sharkipelago commented Jun 7, 2025 •

edited

Loading

sharkipelago Jun 10, 2025 •

edited

Loading