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 a bug in TableExpr.drop #1732

Closed
wants to merge 7 commits into from

Conversation

Projects
None yet
2 participants
@cpcloud
Copy link
Member

commented Mar 16, 2019

This PR fixes a bug where the drop method wasn't using __getitem__ to
project.

__getitem__ calls a function bind_expr that binds a table to the projected
expression.

As a follow-up we should bring this behavior to projection to avoid issues like
this in the future.

One issue with adding a call to ir.bind_expr(self, what) to the projection method is that it breaks some fusion optimizations.

I think that as part of the the post 1.0.0 follow-up work we need to disable and remove all optimizations until we can restructure the compiler flow to separate optimization from expression construction.

cpcloud added some commits Mar 16, 2019

@cpcloud cpcloud force-pushed the cpcloud:fix-drop branch from 3c309d2 to 6ab7520 Mar 17, 2019

@cpcloud cpcloud added this to the 1.0.0 milestone Mar 17, 2019

@cpcloud cpcloud added the bug label Mar 17, 2019

Show resolved Hide resolved ibis/expr/api.py
@kszucs

kszucs approved these changes Mar 18, 2019

Copy link
Member

left a comment

LGTM except the questions in my comments

@kszucs kszucs closed this in 2a4003f Mar 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.