pandas icon indicating copy to clipboard operation
pandas copied to clipboard

BUG: next_workday observance wrongly adds BDay to holidays on weekdays

Open yaoyuan12 opened this issue 9 months ago • 2 comments

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.holiday import Holiday, AbstractHolidayCalendar, next_workday
    
class HolidayCalendar(AbstractHolidayCalendar):
    rules = [
        Holiday('Christmas Day', month=12, day=25, observance=next_workday),
    ]

print(HolidayCalendar().holidays('2024-01-01', '2024-12-31'))

Issue Description

In the example given, Christmas is defined as next_workday after 25th Dec. 2024-12-25 is a Wed; however the print result returns 2024-12-26. It seems to add 1 business day to 2024 Christmas even though it is a normal weekday.

Expected Behavior

Would expect holidays to return 2024-12-25 for observance=next_workday, since 2024-12-25 is a weekday. Indeed this is what returns from a manual implementation of next_workday:

import pandas as pd
from pandas.tseries.holiday import Holiday, AbstractHolidayCalendar, next_workday
    
class HolidayCalendar(AbstractHolidayCalendar):
    rules = [
        Holiday('Christmas Day', month=12, day=25, observance=lambda d: d + pd.offsets.BDay(1) if d.weekday() >= 5 else d),
    ]

print(HolidayCalendar().holidays('2024-01-01', '2024-12-31'))

Installed Versions

INSTALLED VERSIONS

commit : bdc79c146c2e32f2cab629be240f01658cfb6cc2 python : 3.11.5.final.0 python-bits : 64 OS : Linux OS-release : 5.14.0-284.18.1.el9_2.x86_64 machine : x86_64 processor : x86_64

pandas : 2.2.1 numpy : 1.24.4 pytz : 2023.3.post1 dateutil : 2.8.2

yaoyuan12 avatar May 03 '24 16:05 yaoyuan12

Hi @yaoyuan12. The docstring for next_workday says that the function returns next workday used for observances. Hence it will always give the "next working day". You can use weekend_to_monday or next_monday to achieve your desired result.

However, I think the confusion arises from the docs defined here: https://pandas.pydata.org/docs/dev/user_guide/timeseries.html, where the next_workday is defined as move Saturday and Sunday to Monday which contradicts the docstring.

We could either update the docs or try to deprecate the functions that give similar outputs. What do you think @mroeschke?

iofall avatar May 08 '24 19:05 iofall

Hm at the least based on the docs, weekend_to_monday is defined to be the same as next_workday, so that is definitely wrong.

I seem to have an impression that when I wrote the code, I checked and next_workday did indeed produce the same effect as weekend_to_monday, so was that code changed somewhen along the way? Or I could be dreaming.

yaoyuan12 avatar May 09 '24 18:05 yaoyuan12