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 len for interactive exprs that wrap sql #1273

Merged
merged 1 commit into from Oct 13, 2015

Conversation

@llllllllll
Copy link
Member

commented Oct 12, 2015

compute for the sqlbackend would return a select. the len machinery would then try to coerce this into an int and fail with a type error.

@llllllllll llllllllll force-pushed the quantopian:sql-len branch from 8e620c5 to 60f42d8 Oct 12, 2015


def test_interactive_len(sql):
t = Data(sql)
assert len(t) == odo(t.count(), int)

This comment has been minimized.

Copy link
@cpcloud

cpcloud Oct 12, 2015

Member

You can actually just call int(t.count()), but up to you

This comment has been minimized.

Copy link
@llllllllll

llllllllll Oct 12, 2015

Author Member

oh, I didn't realize __int__ was the same. I like the clarity of odo here; however, I don't mind promoting this pattern.

@cpcloud cpcloud added this to the 0.9.0 milestone Oct 12, 2015

@llllllllll llllllllll force-pushed the quantopian:sql-len branch from 435b1e6 to 164b1fa Oct 13, 2015

@cpcloud

This comment has been minimized.

Copy link
Member

commented Oct 13, 2015

@llllllllll Merge when ready

@llllllllll llllllllll force-pushed the quantopian:sql-len branch from 164b1fa to c8155ed Oct 13, 2015

@llllllllll

This comment has been minimized.

Copy link
Member Author

commented Oct 13, 2015

I only needed to resolve the whatsnew but I rebased so I will let this pass before merging.

@cpcloud

This comment has been minimized.

Copy link
Member

commented Oct 13, 2015

cool

cpcloud added a commit that referenced this pull request Oct 13, 2015

Merge pull request #1273 from quantopian/sql-len
BUG: fix len for interactive exprs that wrap sql

@cpcloud cpcloud merged commit 88a9c3b into blaze:master Oct 13, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.