Pandas version checks

  • [X] I have checked that this issue has not already been reported.

  • [X] I have confirmed this bug exists on the latest version of pandas.

  • [ ] I have confirmed this bug exists on the main branch of pandas.

Reproducible Example

from pandas.tseries.offsets import CustomBusinessDay
import pandas_market_calendars as mcal

nyse = mcal.get_calendar('NYSE')
us_bd = CustomBusinessDay(calendar=nyse)

date_range = pd.date_range('2024-12-20', periods=10, freq=us_bd)
correct_schedule = nyse.schedule(start_date='2024-12-23', end_date='2025-01-10')

Issue Description

CustomBusinessDay is not properly showing the correct calendar holiday dates. When displaying date_range in the code above you'll see that 2024-12-25 is being included which was a market holiday and is not shown in correct_schedule in the code above.

Expected Behavior

I would expect for us_bd to respect the dates in 'correct_schedule' and not include market holidays as it has in the past. When running todays date minus 1 us_bd it is showing new years day which also should be excluded and the correct result should yield '2024-12-31'

Installed Versions

INSTALLED VERSIONS

commit : f538741432edf55c6b9fb5d0d496d2dd1d7c2457 python : 3.10.13.final.0 python-bits : 64 OS : Windows OS-release : 10 Version : 10.0.19045 machine : AMD64 processor : Intel64 Family 6 Model 207 Stepping 2, GenuineIntel byteorder : little LC_ALL : None LANG : None LOCALE : English_United States.1252

pandas : 2.2.0 numpy : 1.26.4 pytz : 2023.3 dateutil : 2.8.2 setuptools : 68.2.2 pip : 23.3.1 Cython : 3.0.8 pytest : 8.3.3 hypothesis : None sphinx : None blosc : None feather : None xlsxwriter : None lxml.etree : None html5lib : None pymysql : None psycopg2 : None jinja2 : 3.1.3 IPython : 8.21.0 pandas_datareader : None adbc-driver-postgresql: None adbc-driver-sqlite : None bs4 : 4.12.3 bottleneck : None dataframe-api-compat : None fastparquet : None fsspec : 2023.6.0 gcsfs : None matplotlib : 3.8.2 numba : None numexpr : 2.10.1 odfpy : None openpyxl : 3.1.2 pandas_gbq : None pyarrow : 15.0.0 pyreadstat : None python-calamine : None pyxlsb : None s3fs : None scipy : 1.12.0 sqlalchemy : None tables : None tabulate : None xarray : None xlrd : 2.0.1 zstandard : None tzdata : 2024.1 qtpy : None pyqt5 : None

Comment From: rhshadrach

Thanks for the report. The CustomBusinessDay takes an np.busdaycalendar, but the object your passing is not an instance of this class. It is subsequently ignored. I'm not familiar with the package, but pandas_market_calendar has a date_range function - should you not be using this instead?

On the pandas side, I think we should raise when a calendar that is not an np.busdaycalender is passed instead of silently ignoring it.

Comment From: sreekailash

@benmgrant - This is expected behavior as @rhshadrach pointed out however there should be a message if a wrong object type is passed. The CustomBusinessDay function's calendar parameter takes the busdaycalendar object as an input.

Alternate solution : You can fix this by using the following code snippet -us_bd = nyse.holidays() directly instead of calling the CustomBusinessDay as this returns the right object that can be read by the date_range function.

Complete code:

from pandas.tseries.offsets import CustomBusinessDay
import pandas_market_calendars as mcal

nyse = mcal.get_calendar('NYSE')
us_bd = nyse.holidays()

date_range = pd.date_range('2024-12-23', periods=10, freq=us_bd)
correct_schedule = nyse.schedule(start_date='2024-12-23', end_date='2025-01-10')

Hope this helps in the mean time. I will create a PR to raise a warning message when a wrong object is passed to CustomBusinessDay

Comment From: rhshadrach

I will create a PR to raise a warning message when a wrong object is passed to CustomBusinessDay

I think we should raise an error, not a warning message.

Comment From: qscgy

I will create a PR to raise a warning message when a wrong object is passed to CustomBusinessDay

I think we should raise an error, not a warning message.

I tried this. It ends up failing some unit tests that pass a non-np.busdaycalendar object to CustomBusinessDay, but the documentation for CustomBusinessDay lists the type of calendar as np.busdaycalendar, so another change is needed elsewhere. I think we should raise a warning for now and open another PR for this inconsistency.

Comment From: qscgy

The underlying code that extracts the holidays from calendar is in _get_calendar() in pandas/_libs/tslibs/offsets.pyx:

if isinstance(calendar, np.busdaycalendar):
        if not holidays:
            holidays = tuple(calendar.holidays)
        elif not isinstance(holidays, tuple):
            holidays = tuple(holidays)
        else:
            # trust that calendar.holidays and holidays are
            # consistent
            pass
        return calendar, holidays

    if holidays is None:
        holidays = []
    try:
        holidays = holidays + calendar.holidays().tolist()
    except AttributeError:
        pass

It is written to handle a calendar that isn't an np.busdaycalendar but has a .holidays() method. Problem is, the try-except block presumably is meant to check for .holidays() existing, but it also excepts when the method does exist but whatever is returned by .holidays() doesn't have a tolist() method.

In this situation, nyse.holidays() is a CustomBusinessDay, but CustomBusinessDay itself doesn't have a tolist() method. So it seems like the best solution is to either add a tolist() method to it or one of its superclasses, or fix pandas_market_calendars so that nyse.holidays() returns a DatetimeIndex as done by the built-in holiday calendars in pandas.

Comment From: rhshadrach

It is written to handle a calendar that isn't an np.busdaycalendar

This goes back to:

https://github.com/jbrockmendel/pandas/commit/68f626872676604f34190580df23d9653a389e03#diff-4fa7fdc555f07dc10c337a388c6a8a8f357bd861cd31171e5c2daa3ca673665eR815

where it also accepted pd.HolidayCalendar. This was removed in #46920. I think the intention here is to only support np.busdaycalendar.

Comment From: tomascortes

So, if I understand correctly, it should be removed and should only accept np.busdaycalendar?

Comment From: rhshadrach

@tomascortes - yes, that is my current understanding. If there are any other thoughts they are always welcome!

Comment From: christinepuk

@VishalSindham are you working on this? I would like to take it otherwise

Comment From: Bala-Sid

take

Comment From: mindofVishesh

take

Comment From: yash4agr

Hi @mindofVishesh are you working on this? I would like to take it otherwise

Comment From: yash4agr

take

Comment From: Nivetha200111

Hey everyone,

I’d like to work on #60647 (“CustomBusinessDay not respecting calendar”). I’m planning to have CustomBusinessDay throw a TypeError if the passed-in object isn’t an np.busdaycalendar, and them add a unit test in tests/tseries/test_offsets.py to cover it.

Is this still up for grabs? If so, could someone please assign it to me? Thanks!

Comment From: prblydv

Hi! I’d like to work on this bug – assigning myself now.