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

Adds a Shift expression #1266

Merged
merged 15 commits into from Oct 13, 2015

Conversation

Projects
None yet
2 participants
@cpcloud
Member

cpcloud commented Oct 7, 2015

No description provided.

# if we are not shifting or we are already an Option type then return
# the child's schema
if not self.n or isinstance(measure, Option):

This comment has been minimized.

@llllllllll

llllllllll Oct 8, 2015

Member

should a shift(expr, 0) just return expr?

This comment has been minimized.

@cpcloud

cpcloud Oct 12, 2015

Member

I'm going to leave it as is for now. If this becomes a bottleneck in the future then it's very easy to change

@copydoc(Shift)
def shift(expr, n):
if not isinstance(n, (numbers.Integral, np.integer)):

This comment has been minimized.

@llllllllll

llllllllll Oct 8, 2015

Member

is there a reason we cannot allow expressions for n. like, a.shift(b.count())?

This comment has been minimized.

@cpcloud

cpcloud Oct 12, 2015

Member

No, but I'd like to do that in a separate PR

This comment has been minimized.

@llllllllll
Parameters
----------
expr : Expr
The expression to shift. This expression's dshape should be columnar

This comment has been minimized.

@llllllllll

llllllllll Oct 11, 2015

Member

Should we typecheck in the shift constructor?

This comment has been minimized.

@cpcloud

cpcloud Oct 12, 2015

Member

no, this done by the dshape methods/property checking in __getattr__

This comment has been minimized.

@llllllllll

llllllllll Oct 12, 2015

Member

Should we not expose the shift toplevel function also?

@cpcloud cpcloud force-pushed the cpcloud:shift branch from f6d673c to 616a491 Oct 12, 2015

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

@cpcloud cpcloud self-assigned this Oct 12, 2015

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

Merge pull request #1266 from cpcloud/shift
Adds a Shift expression

@cpcloud cpcloud merged commit 6c67b5f into blaze:master Oct 13, 2015

1 check passed

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

@cpcloud cpcloud deleted the cpcloud:shift branch Oct 13, 2015

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