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 frame column validation in Python 2.7.5 (#1771) #1954

Merged
merged 1 commit into from Sep 25, 2017

Conversation

Projects
None yet
4 participants
@tibkiss
Contributor

tibkiss commented Sep 19, 2017

No description provided.

@coveralls

This comment has been minimized.

coveralls commented Sep 19, 2017

Coverage Status

Coverage remained the same at 87.19% when pulling 381cbe1 on tibkiss:fix/unexp_frame_columns_in_us_equity_pricing into 77922dd on quantopian:master.

1 similar comment
@coveralls

This comment has been minimized.

coveralls commented Sep 19, 2017

Coverage Status

Coverage remained the same at 87.19% when pulling 381cbe1 on tibkiss:fix/unexp_frame_columns_in_us_equity_pricing into 77922dd on quantopian:master.

@@ -915,7 +915,8 @@ def _write(self, tablename, expected_dtypes, frame):
np.array([], dtype=list(expected_dtypes.items())),
)
else:
if frozenset(frame.columns) != viewkeys(expected_dtypes):
if frozenset(frame.columns) != \
frozenset(viewkeys(expected_dtypes)):

This comment has been minimized.

@ssanderson

ssanderson Sep 19, 2017

Member

Is there still a point in calling viewkeys here if we're going to pass the input to frozenset anyway?

This comment has been minimized.

@tibkiss

tibkiss Sep 24, 2017

Contributor

good point, updated the PR accordingly

@ssanderson

This comment has been minimized.

Member

ssanderson commented Sep 19, 2017

@tibkiss this seems reasonable to me. I assume this is working around https://bugs.python.org/issue8404 ? Just had one small perf-related question.

@richafrank

This comment has been minimized.

Member

richafrank commented Sep 19, 2017

Looks like a fix for #1641

@tibkiss

This comment has been minimized.

Contributor

tibkiss commented Sep 24, 2017

@richafrank / @ssanderson : Thanks for the quick review & sorry for the late response.

@ssanderson : Even though your linked issue looks really similar I don't think it is exactly the same. In issue8404 the affectedVersion is a Python RC and the problem I'm workarounding here is present on python 2.7.5.

Nonetheless this fix solves #1771 & #1641 (which are essentially the same).

@coveralls

This comment has been minimized.

coveralls commented Sep 24, 2017

Coverage Status

Coverage remained the same at 87.203% when pulling 1448bd9 on tibkiss:fix/unexp_frame_columns_in_us_equity_pricing into 64af3d3 on quantopian:master.

@richafrank

This comment has been minimized.

Member

richafrank commented Sep 25, 2017

CI tests pass, and this looks safe to me. Merging.

@richafrank richafrank merged commit ec94e5a into quantopian:master Sep 25, 2017

2 checks passed

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

This comment has been minimized.

Member

richafrank commented Sep 25, 2017

Thanks @tibkiss !

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