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

BUG: Fix an issue where history fails on a single 1m price history. #354

Merged
merged 1 commit into from
Jun 25, 2014

Conversation

ssanderson
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.97%) when pulling e68b87d on 1m_history_bugfix into 1b571a5 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.97%) when pulling 929e32b on 1m_history_bugfix into 1b571a5 on master.

@ehebert
Copy link
Contributor

ehebert commented Jun 25, 2014

What was the issue that is being fixed? i.e. what was it about the call to get_open_and_close that was causing the issue? (Could you answer that via updating the commit message?)

Small nit-picky thing is that the first sentence/subject of the commit message would be better if it were shorter than 72 chars.

@ssanderson
Copy link
Contributor Author

Updated with a more thorough explanation.

The bug occurred because there is a special case in the initial window setup
code for handling the case where only a length-1 history is used for a given
frequency.  Previously, the code was incorrectly calculating the period end
using a hard-coded expression for the end of the day (the correct behavior for
a length-1 '1d' history), and then using the frequency object to calculate the
period start for the window.  In the case of length 1 '1m' data, this resulted
in an initial window whose start and end was the last minute of the day rather
than the first minute of the day.  For non-price fields, this error doesn't
matter, because the window is only used for rolling digests (which doesn't
happen when there's only a length-1 history), and for the forward-filling logic
(which only happens on price fields).  For a length-1 '1m' price, however, the
incorrect window causes us to attempt to forward-fill an empty panel, resulting
in an IndexError when we do an iloc[0] on a length-0 axis.
@ehebert
Copy link
Contributor

ehebert commented Jun 25, 2014

Thanks, the thorough explanation is great. /signoff

@coveralls
Copy link

Coverage Status

Coverage increased (+2.97%) when pulling 6a2ee7c on 1m_history_bugfix into 1b571a5 on master.

@ssanderson ssanderson merged commit 6a2ee7c into master Jun 25, 2014
@coveralls
Copy link

Coverage Status

Coverage increased (+2.97%) when pulling 6a2ee7c on 1m_history_bugfix into 1b571a5 on master.

@ehebert ehebert deleted the 1m_history_bugfix branch February 2, 2015 20:10
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