Skip to content

Allow tz-aware inputs in Holiday.dates #9824

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
merged 1 commit into from
Apr 8, 2015
Merged

Allow tz-aware inputs in Holiday.dates #9824

merged 1 commit into from
Apr 8, 2015

Conversation

ssanderson
Copy link
Contributor

closes #9825

Previously, passing tz-aware inputs to Holiday.dates would always result in an error because the the input start_date was converted in a way that destroyed timezone information. This PR makes that conversion correctly preserve tz info, and converts several of the existing tests to run with both tz-aware and tz-naive inputs.

Noticed while working on a new trading calendar implementation here: quantopian/zipline#556.

@ssanderson ssanderson mentioned this pull request Apr 7, 2015
Closed
@jreback
Copy link
Contributor

jreback commented Apr 7, 2015

your tests don't check for a holiday with a timezone. wasn't this the use case?

@jreback jreback added the Timezones Timezone data dtype label Apr 7, 2015
@ssanderson
Copy link
Contributor Author

@jreback the specific issue solved here is that calling holiday.dates(start, end) with tz-aware start and end raises an error because dates attempts to construct a DatetimeIndex with a tz-naive start and the supplied end. The fix here just propagates the timezone of the supplied start so that the call succeeds if both inputs are tz-aware and in the same timezone. That's tested at https://github.com/pydata/pandas/pull/9824/files#diff-96b913a0de65ff4f70fc5bde0543e9a8R62 by running the same checks that were performed previously, but with dates that are localized to UTC. The substantive content of the testing changes are in ssanderson@8ae03df. The rest of the diff is just refactoring to make it easier to run the same check twice, once with tz-naive inputs, and once with tz-aware inputs.

@jreback
Copy link
Contributor

jreback commented Apr 8, 2015

@ssanderson ok, I see, then ok!

pls squash and good to go

@ssanderson
BUG: Preserve tz of start_date in Holiday.dates
0a216f4 
Previously, the timezone of only the start_date was lost, causing the
subsequent call to `DatetimeIndex` to fail if a both start_date and
end_date were tz-aware.
@ssanderson
Copy link
Contributor Author

@jreback squashed into one commit and rebased with master. I assume that's what you meant?

jreback added a commit that referenced this pull request Apr 8, 2015
@jreback
Merge pull request #9824 from ssanderson/holiday-allow-tz
fa06aeb 
Allow tz-aware inputs in Holiday.dates
@jreback jreback merged commit fa06aeb into pandas-dev:master Apr 8, 2015
@jreback
Copy link
Contributor

jreback commented Apr 8, 2015

@ssanderson yep

thanks for the PR!

@jreback jreback added this to the 0.16.1 milestone Apr 8, 2015
@ssanderson
Copy link
Contributor Author

@jreback great! #9825 can be closed (I mistyped in my issue description).

@jreback
Copy link
Contributor

jreback commented Apr 8, 2015

fixed thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Enhancement Timezones Timezone data dtype
Projects
None yet
Milestone
0.16.1
Development

Successfully merging this pull request may close these issues.

Holiday.dates always fails with tz-aware input
2 participants
@ssanderson @jreback