Skip to content
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

jbredeche
Copy link
Member

@jbredeche 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.

…he calendar.

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

Price is still forward filled, unchanged.
@coveralls
Copy link

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')
Copy link
Contributor

@ehebert ehebert Sep 15, 2016

Choose a reason for hiding this comment

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

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,
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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
@jbredeche jbredeche deleted the who-said-you-could-forward-fill-stop-that branch September 15, 2016 19:41
bartosh pushed a commit to bartosh/zipline that referenced this pull request Sep 27, 2016
…-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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants