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

PERF: Be smarter about counting the number of minutes across a contiguous bunch of sessions. #1497

Merged
merged 1 commit into from Sep 19, 2016

Conversation

Projects
None yet
4 participants
@jbredeche
Member

jbredeche commented Sep 19, 2016

Much, much, much faster this way.

@richafrank

This comment has been minimized.

Member

richafrank commented Sep 19, 2016

Looking at the repr of NYSE.minutes_per_session, I see:

1990-01-02 00:00:00+00:00    389
1990-01-03 00:00:00+00:00    389
1990-01-04 00:00:00+00:00    389
1990-01-05 00:00:00+00:00    389
1990-01-08 00:00:00+00:00    389
1990-01-09 00:00:00+00:00    389
1990-01-10 00:00:00+00:00    389
1990-01-11 00:00:00+00:00    389
1990-01-12 00:00:00+00:00    389
1990-01-15 00:00:00+00:00    389
1990-01-16 00:00:00+00:00    389
1990-01-17 00:00:00+00:00    389
...

Is that expected?

df: A dataframe containing the number of minutes per session.
"""
diff = self.schedule.market_close - self.schedule.market_open
return diff.astype('timedelta64[m]')

This comment has been minimized.

@richafrank

richafrank Sep 19, 2016

Member

Instead of the calendar exposing a dataframe to consumers, so now we're not in the managed land of sessions and labels anymore, would it make sense to expose a function that takes a list of sessions and does the slicing, i.e. move the call site code here?

This comment has been minimized.

@jbredeche

jbredeche Sep 19, 2016

Member

agreed, will make it so.

@jbredeche

This comment has been minimized.

Member

jbredeche commented Sep 19, 2016

@richafrank 389 is not expected. Am pushing a fix for that.

@jbredeche jbredeche force-pushed the you-count-way-too-slowly branch from afda28b to b455731 Sep 19, 2016

int: The total number of minutes for the contiguous chunk of sessions.
between start_session and end_session, inclusive.
"""
return sum(self._minutes_per_session[start_session:end_session])

This comment has been minimized.

@richafrank

richafrank Sep 19, 2016

Member

Is there a vectorized sum that we should use here?

This comment has been minimized.

@jbredeche

jbredeche Sep 19, 2016

Member

yup, done

@jbredeche jbredeche force-pushed the you-count-way-too-slowly branch from b455731 to c203ec9 Sep 19, 2016

@jbredeche jbredeche force-pushed the you-count-way-too-slowly branch from c203ec9 to 87eb875 Sep 19, 2016

@coveralls

This comment has been minimized.

coveralls commented Sep 19, 2016

Coverage Status

Coverage decreased (-0.4%) to 86.251% when pulling 87eb875 on you-count-way-too-slowly into 95e07f2 on master.

@coveralls

This comment has been minimized.

coveralls commented Sep 19, 2016

Coverage Status

Coverage increased (+0.006%) to 86.675% when pulling 87eb875 on you-count-way-too-slowly into 95e07f2 on master.

@jbredeche jbredeche merged commit 77e50be into master Sep 19, 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 you-count-way-too-slowly branch Sep 19, 2016

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