Skip to content

BUG: Fix creating DatetimeIndex with BusinessHour Frequency #50530

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.

Sign up for GitHub

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

Jump to bottom
Merged
mroeschke merged 13 commits into from
Jan 4, 2023
Merged

BUG: Fix creating DatetimeIndex with BusinessHour Frequency #50530

mroeschke merged 13 commits into from
Jan 4, 2023

Conversation

@DavidKleindienst
Copy link
Contributor

@DavidKleindienst DavidKleindienst commented Jan 2, 2023 •
edited
Loading

DatetimeIndex creating with BusinessHour Frequency would fail whenever the DatetimeIndex did not include a Timestamp that was also an opening time.

The cause of the issue was a bug where adding/subtracting 0 * BusinessHour gave wrong results:

import pandas as pd
import datetime as dt
dt.datetime(2020,1,1,10) -0 * pd.offsets.BusinessHour()

returns Timestamp('2020-01-02 09:00:00') rather than the expected Timestamp('2020-01-01 10:00:00')

This PR fixes the problem and adds appropriate assertions to the unit-tests

@DavidKleindienst DavidKleindienst changed the title Fix creating DatetimeIndex with BusinessHour Frequency BUG: Fix creating DatetimeIndex with BusinessHour Frequency Jan 3, 2023
mroeschke
mroeschke reviewed Jan 3, 2023
View reviewed changes
doc/source/whatsnew/v1.5.3.rst Outdated
- Bug when chaining several :meth:`.Styler.concat` calls, only the last styler was concatenated (:issue:`49207`)
- Fixed bug when instantiating a :class:`DataFrame` subclass inheriting from ``typing.Generic`` that triggered a ``UserWarning`` on python 3.11 (:issue:`49649`)
-
- Fixed bug in :class:`BusinessHour` that could cause creation of ``DatetimeIndex`` to fail (:issue:`#49835`)
Copy link
Member

@mroeschke mroeschke Jan 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Fixed bug in :class:`BusinessHour` that could cause creation of ``DatetimeIndex`` to fail (:issue:`#49835`)
- Fixed bug in :class:`BusinessHour` that could cause creation of :class:`DatetimeIndex` to fail (:issue:`49835`)
  1. Could you move this to v2.0.0.rst?
  2. Could you make this description more specific too?

Copy link
Contributor Author

@DavidKleindienst DavidKleindienst Jan 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've written it like this now, is that ok?
- Bug in :class:`BusinessHour` would cause creation of :class:`DatetimeIndex` to fail when no opening hour was included in the index (:issue:`#49835`)

Thanks for reviewing!

mroeschke
mroeschke reviewed Jan 3, 2023
View reviewed changes
mroeschke
mroeschke reviewed Jan 3, 2023
View reviewed changes
@mroeschke mroeschke added Frequency DateOffsets Datetime Datetime data dtype labels Jan 3, 2023
mroeschke
mroeschke reviewed Jan 4, 2023
View reviewed changes
@mroeschke mroeschke added this to the 2.0 milestone Jan 4, 2023
@mroeschke mroeschke merged commit c6d0bc0 into pandas-dev:main Jan 4, 2023
@mroeschke
Copy link
Member

mroeschke commented Jan 4, 2023

Great! Thanks @DavidKleindienst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mroeschke mroeschke mroeschke approved these changes

Assignees

No one assigned

Labels

Datetime Datetime data dtype Frequency DateOffsets

Projects

None yet

Milestone

2.0

Development

Successfully merging this pull request may close these issues.

BUG: DatetimeIndex creation with BusinessHour frequency fails when first hour of the day is not included as Timestamp

2 participants

@DavidKleindienst @mroeschke