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

ENH: Make reader.get_value raise NoDataOnDate if the date is not in the calendar. #1488

Merged
merged 1 commit into from Sep 15, 2016

Conversation

Projects
None yet
3 participants
@jbredeche
Member

jbredeche commented Sep 15, 2016

Previously, at least the minute bar reader was forward filling (using the previous market minute).

DataPortal now catches the NoDataOnDate exception and returns nan for OHLC and 0 for V.

Price is still forward filled, unchanged.

ENH: Make reader.get_value raise NoDataOnDate if the date is not in t…
…he calendar.

DataPortal now catches the NoDataOnDate exception and returns nan for
OHLC and 0 for V.

Price is still forward filled, unchanged.
@coveralls

This comment has been minimized.

coveralls commented Sep 15, 2016

Coverage Status

Coverage increased (+0.002%) to 86.694% when pulling ae0d41a on who-said-you-could-forward-fill-stop-that into e1b27c4 on master.

"futures trading is open. Result should be 0.")
with self.assertRaises(NoDataOnDate):
self.reader.get_value(1, tday, 'close')

This comment has been minimized.

@ehebert

ehebert Sep 15, 2016

Member

Could we retain the error message about how NYSE is on one calendar but not the other?

@@ -112,7 +128,7 @@ def find_last_traded_position_internal(
minute_pos = int_min(
find_position_of_minute(market_opens, market_closes, end_minute,

This comment has been minimized.

@ehebert

ehebert Sep 15, 2016

Member

I see how avoiding the extra searchsorted is helpful, but I worry about making the behavior of find_position_of_minute_minute adjustable for ffill or not; leaving the function as simple as possible.

Another option would be to put the handling of the NoDataOnDate in this scope (which is focused on finding the last date.)

Something like:

try:
    minute_pos = int_min(
         find_position_of_minute(market_opens, market_closes, end_minute,
                                                minutes_per_day)
except NoDataOnDate:
    prev_close_loc = searchsorted(market_closes, end_minute)
    minute_pos = (prev_close_loc + 1)  * minute_per_day) - 1

This comment has been minimized.

@jbredeche

jbredeche Sep 15, 2016

Member

hm, true. But then you're essentially replicating find_position_of_minute's logic here (the searchsorted call, plus the math).

I think I prefer it the way it is.

@jbredeche jbredeche merged commit c77f2b9 into master Sep 15, 2016

2 checks passed

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

@jbredeche jbredeche deleted the who-said-you-could-forward-fill-stop-that branch Sep 15, 2016

bartosh pushed a commit to bartosh/zipline that referenced this pull request Sep 27, 2016

Merge pull request quantopian#1488 from quantopian/who-said-you-could…
…-forward-fill-stop-that

ENH: Make reader.get_value raise NoDataOnDate if the date is not in the calendar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment