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: Avoid unnecessary work with missing data. #1451

Merged
merged 1 commit into from Aug 31, 2016

Conversation

Projects
None yet
3 participants
@jbredeche
Member

jbredeche commented Aug 30, 2016

No description provided.

@jbredeche jbredeche force-pushed the zero-means-zero branch from 2991521 to 6208563 Aug 30, 2016

# if asset 1 started on 2015-01-03 but its first trade is 2015-01-06
# 10:31 AM US/Eastern, this dict would store {1: 1420558260000000000},
# which is the nanosecond epoch of that date.
self._known_zero_volume_dict = {}

This comment has been minimized.

@ehebert

ehebert Aug 30, 2016

Member

I would recommend storing these as the minute since epoch, since that is what the frame of reference of the other values you are comparing against is.
This would avoid an extra / NANOS_IN_MINUTE call every time it is used.

start_date_minute = asset.start_date.value / NANOS_IN_MINUTE
dt_minute = dt.value / NANOS_IN_MINUTE
if asset.sid in self._known_zero_volume_dict:

This comment has been minimized.

@ehebert

ehebert Aug 30, 2016

Member

Using try/KeyError is preferable. Avoids an extra hash and lookup into dictionary.

e.g.

try:
    earliest_dt_to_search = self._known_zero_volume_dict[asset.sid] / NANOS_IN_MINUTE
except KeyError:
    earliest_dt_to_search = start_date_minute
if pos == -1:
# if we didn't find any volume before this dt, save it to avoid
# work in the future.
if asset.sid not in self._known_zero_volume_dict:

This comment has been minimized.

@ehebert

ehebert Aug 30, 2016

Member
try:
    self._known_zero_volume_dict[asset.sid] = max(
        dt.value,
        self._known_zero_volume_dict[asset.sid]
   )
except KeyError:
    self._known_zero_volume_dict[asset.sid] = dt.value

@jbredeche jbredeche force-pushed the zero-means-zero branch from fba081f to dd53028 Aug 30, 2016

@coveralls

This comment has been minimized.

coveralls commented Aug 30, 2016

Coverage Status

Coverage increased (+0.009%) to 86.139% when pulling 6208563 on zero-means-zero into 1984d13 on master.

@jbredeche jbredeche force-pushed the zero-means-zero branch from dd53028 to 972f05b Aug 30, 2016

@coveralls

This comment has been minimized.

coveralls commented Aug 30, 2016

Coverage Status

Changes Unknown when pulling 972f05b on zero-means-zero into * on master*.

@coveralls

This comment has been minimized.

coveralls commented Aug 30, 2016

Coverage Status

Coverage increased (+0.01%) to 86.141% when pulling 972f05b on zero-means-zero into 7fe1a56 on master.

2 similar comments
@coveralls

This comment has been minimized.

coveralls commented Aug 30, 2016

Coverage Status

Coverage increased (+0.01%) to 86.141% when pulling 972f05b on zero-means-zero into 7fe1a56 on master.

@coveralls

This comment has been minimized.

coveralls commented Aug 30, 2016

Coverage Status

Coverage increased (+0.01%) to 86.141% when pulling 972f05b on zero-means-zero into 7fe1a56 on master.

@coveralls

This comment has been minimized.

coveralls commented Aug 30, 2016

Coverage Status

Coverage increased (+0.01%) to 86.141% when pulling 972f05b on zero-means-zero into 7fe1a56 on master.

@coveralls

This comment has been minimized.

coveralls commented Aug 31, 2016

Coverage Status

Coverage increased (+0.01%) to 86.141% when pulling 972f05b on zero-means-zero into 7fe1a56 on master.

@jbredeche jbredeche merged commit 1631d2a into master Aug 31, 2016

2 checks passed

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

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

Merge pull request quantopian#1451 from quantopian/zero-means-zero
ENH: Avoid unnecessary work with missing data.

@richafrank richafrank deleted the zero-means-zero branch Nov 10, 2016

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