Skip to content

Bug: CustomBusinessHour bug with holidays #16867

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

Closed
ustcscgyer opened this issue Jul 10, 2017 · 5 comments · Fixed by #41674
Closed

Bug: CustomBusinessHour bug with holidays #16867

ustcscgyer opened this issue Jul 10, 2017 · 5 comments · Fixed by #41674
Labels
good first issue Needs Tests Unit test(s) needed to prevent regressions
Milestone

Comments

@ustcscgyer
Copy link

Code Sample, a copy-pastable example if possible

# Your code here
import pandas as pd # My pandas version is 0.19.2

from pandas.tseries.holiday import USFederalHolidayCalendar
from pandas.tseries.offsets import CustomBusinessHour
import datetime as dt
bhour_us = CustomBusinessHour(calendar=USFederalHolidayCalendar())

# Friday before MLK Day
t0 = dt.datetime(2014, 1, 17, 15)

# This is still right
# Expected Result: Timestamp('2014-01-21 14:00:00')
# print(t0 + bhour_us * 7 )

# This is wrong
# Expected Result: Timestamp('2014-01-21 15:00:00')
print(t0 + bhour_us * 8)
# But the code generates: Timestamp('2014-01-20 15:00:00')

Problem description

I used a Timestamp one day prior to a Holiday as t0. When offset it by number of hours that is less than the number of hours of a business day, pandas correctly accounts for the holidays, but when the number of hours in the offset is equal or larger than number of hours of a business day, pandas gives one day prior to the correct result.

Expected Output

Timestamp('2014-01-21 15:00:00')

Output of pd.show_versions()

INSTALLED VERSIONS

commit: None
python: 3.4.5.final.0
python-bits: 64
OS: Windows
OS-release: 10
machine: AMD64
processor: Intel64 Family 6 Model 78 Stepping 3, GenuineIntel
byteorder: little
LC_ALL: None
LANG: None
LOCALE: None.None

pandas: 0.19.2
nose: 1.3.7
pip: 9.0.1
setuptools: 27.2.0
Cython: 0.25.2
numpy: 1.11.3
scipy: 0.18.1
statsmodels: None
xarray: None
IPython: 5.1.0
sphinx: 1.5.1
patsy: 0.4.1
dateutil: 2.6.0
pytz: 2016.10
blosc: None
bottleneck: 1.2.0
tables: 3.2.2
numexpr: 2.6.1
matplotlib: 2.0.0
openpyxl: 2.4.1
xlrd: 1.0.0
xlwt: 1.2.0
xlsxwriter: 0.9.6
lxml: 3.7.2
bs4: 4.5.3
html5lib: None
httplib2: None
apiclient: None
sqlalchemy: 1.1.5
pymysql: None
psycopg2: None
jinja2: 2.9.4
boto: 2.45.0
pandas_datareader: None

@TomAugspurger
Copy link
Contributor

Yeah, these two look very strange:

In [34]: t0 + bhour_us * 7
Out[34]: Timestamp('2014-01-21 14:00:00')

In [35]: t0 + bhour_us * 8
Out[35]: Timestamp('2014-01-20 15:00:00')

Would welcome a PR if you have time to dig into what's going on.

@TomAugspurger TomAugspurger modified the milestones: 0.21.0, Next Major Release Jul 12, 2017
@alanbato
Copy link
Contributor

Claiming this bug :)

@alanbato
Copy link
Contributor

So far I could identify the offending part of the code in offsets.py:837

if bd != 0:
    skip_bd = BusinessDay(n=bd)
    # midnight business hour may not on BusinessDay
    if not self.next_bday.onOffset(other):
        remain = other - self._prev_opening_time(other)
        other = self._next_opening_time(other + skip_bd) + remain
    else:
        other = other + skip_bd

The call to self.next_bday.onOffset(other) returns True, so the code else clause returns other + skip_bd as the result of the offsetting. However, if the code in the if clause executed, it would return the correct result.

If anyone has any ideas on how to refactor/fix this part, I'd be happy to hear them.

@mroeschke
Copy link
Member

This looks to be fixed on master. Could use a test

In [4]: import pandas as pd # My pandas version is 0.19.2
   ...:
   ...: from pandas.tseries.holiday import USFederalHolidayCalendar
   ...: from pandas.tseries.offsets import CustomBusinessHour
   ...: import datetime as dt
   ...: bhour_us = CustomBusinessHour(calendar=USFederalHolidayCalendar())
   ...:
   ...: # Friday before MLK Day
   ...: t0 = dt.datetime(2014, 1, 17, 15)

In [5]: print(t0 + bhour_us * 8)
2014-01-21 15:00:00

@mroeschke mroeschke added good first issue Needs Tests Unit test(s) needed to prevent regressions and removed Frequency DateOffsets Datetime Datetime data dtype labels Apr 1, 2020
@tqa236
Copy link
Contributor

tqa236 commented Jun 22, 2020

I can help to add the tests. Can you tell me which file I should add them to?

@mroeschke mroeschke mentioned this issue May 26, 2021
8 tasks
@jreback jreback modified the milestones: Contributions Welcome, 1.3 May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Needs Tests Unit test(s) needed to prevent regressions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants