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

Fix microoptimizations #1568

Merged
merged 3 commits into from Oct 28, 2016
Merged

Fix microoptimizations #1568

merged 3 commits into from Oct 28, 2016

Conversation

ssanderson
Copy link
Contributor

No description provided.

Rename _get_daily_window_for_sids to _get_daily_window_data.
Rename _get_minute_window_for_assets to _get_minute_window_data.
Rename _get_daily_data to get_daily_spot_value.
Copy link
Contributor

@llllllllll llllllllll left a comment

Choose a reason for hiding this comment

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

changes look good

days_for_window,
end_dt,
field_to_use):
ends_at_midnight = (0 == end_dt.hour == end_dt.minute)
Copy link
Contributor

Choose a reason for hiding this comment

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

style nit: could we move 0 to the far rhs?

@@ -748,6 +748,9 @@ def get_spot_value(self, asset, field, dt, data_frequency):
# otherwise just return a fixed value
return int(asset)

# XXX: These aren't actually the methods that are used by the superclasses,
# so these don't do anything, and this class will likely produce unexpected
# results for history().
def _get_daily_window_for_sid(self, asset, field, days_in_window,
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just remove this method if it isn't used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured the original intent here would be more helpful for whoever eventually gets bitten by this. I just happened to notice that these aren't used because I was looking at renaming methods in data portal.

Copy link
Contributor

Choose a reason for hiding this comment

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

kk

@coveralls
Copy link

coveralls commented Oct 28, 2016

Coverage Status

Coverage decreased (-0.004%) to 86.973% when pulling 21a3f1a on fix-microoptimizations into 2c819a6 on master.

@coveralls
Copy link

coveralls commented Oct 28, 2016

Coverage Status

Coverage decreased (-0.004%) to 86.973% when pulling 21a3f1a on fix-microoptimizations into 2c819a6 on master.

@ssanderson ssanderson merged commit 5ecb5c3 into master Oct 28, 2016
@ssanderson ssanderson deleted the fix-microoptimizations branch October 28, 2016 18:57
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