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

Fix bug in TradingCalendar initialization #1802

Merged
merged 1 commit into from May 22, 2017

Conversation

Projects
None yet
3 participants
@Peque
Contributor

Peque commented May 22, 2017

A TypeError exception was raised with message "Cannot join tz-naive with
tz-aware DatetimeIndex". Removing old unnecessary workaround in
holidays_at_time function (Pandas already fixed that before 0.18)
fixes this issue.

Should fix #1801.

Fix bug in TradingCalendar initialization
A TypeError exception was raised with message "Cannot join tz-naive with
tz-aware DatetimeIndex". Removing old unnecessary workaround in
`holidays_at_time` function (Pandas already fixed that before 0.18)
fixes this issue.
@coveralls

This comment has been minimized.

coveralls commented May 22, 2017

Coverage Status

Coverage remained the same at 87.547% when pulling 64f991b on Peque:calendar_bug into c2e0114 on quantopian:master.

@ssanderson

This comment has been minimized.

Member

ssanderson commented May 22, 2017

@Peque I'm taking a look at this now.

class CalendarStartEndTestCase(TestCase):
def test_start_end(self):

This comment has been minimized.

@ssanderson

ssanderson May 22, 2017

Member

👍 thanks for the test!

@@ -889,11 +889,7 @@ def days_at_time(days, t, tz, day_offset=0):
def holidays_at_time(calendar, start, end, time, tz):
return days_at_time(
calendar.holidays(

This comment has been minimized.

@ssanderson

ssanderson May 22, 2017

Member

👍 on removing this workaround.

FWIW, I think the root cause of the failure is actually https://github.com/quantopian/zipline/blob/master/zipline/utils/calendars/trading_calendar.py#L876. The contract for days_at_time is that it's supposed to always return a DatetimeIndex localized to UTC. We weren't doing that in this case, however, because there were no holidays in the NYSE calendar between 2010-01-03 and 2010-01-10.

I'll fix the empty array case in a separate change.

@ssanderson ssanderson merged commit beba243 into quantopian:master May 22, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Peque Peque deleted the Peque:calendar_bug branch May 23, 2017

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