-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
ENH: allow in-line expression assignment with df.eval #5343
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
Conversation
@cpcloud what do you think? |
return self.visit(expr, **kwargs) | ||
|
||
# allow a single assignment | ||
if isinstance(expr, ast.Assign): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason not to put this in a visit_Assign
method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't try that because we have one which basically switches the Assign to a Compare and I think its needed for that? (e.g. if its on the rhs of an expression). I also wanted to ensure only a single assignment, e.g. a = b = c
is invalid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's only for PyTables, though. Not meaning to be nitpicky.
What happens with overlapping locals? |
you mean like
|
yep |
I actually don't think its a problem because in this case the local doesn't matter my question to use is, I am using |
I think this works as expected (put tests in for this)
|
If you pass in |
I wonder if an |
I suppose |
FWIW I would like to eventually gut |
At some point I should also add |
|
TST: tests for local name overlaps ENH: moved assign to visit_Assign from visit_Module
ok...now use |
on the resolvers. I think that I need to have an explicit way of having the target recorded by |
TST: addtional tests for multiple assignment, targets ENH: add target to Scope, use instead of resolvers
@cpcloud all updated now....take a look when you have a chance, added a short note in the docs too. |
orig_df = df.copy() | ||
|
||
# multiple assignees | ||
self.assertRaises(SyntaxError, df.eval, 'd c = a + b') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be switched with test right below? This is not valid Python syntax while the one below is valid Python syntax, but not valid eval
syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check but iirc it fails in the assign block with a left hand side that is a list with a length of 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strange ... that should simply throw the usual syntax error and not do any parsing outside of Python
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right 'd c = a + b'
raises a SyntaxError in fix_missing_locations....I think I meant the a = b = c
, which ends up having multiple assignment nodes (which I raise a Syntax Error); though in theory in the future you could handle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can u change the test to make sure the multiple assignment fails? again not meaning to be a troll....just want to make sure the correct error is checked
otherwise look ok @cpcloud ? |
yep looks okay |
# multiple assignment | ||
df = orig_df.copy() | ||
df.eval('c = a + b') | ||
self.assertRaises(SyntaxError, df.eval, 'c = a = b') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cpcloud did you mean something additional besides this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh nope! sorry didn't see that 👍
ENH: allow in-line expression assignment with df.eval
This was a relatively easy extension of eval to allow in-line creation/assignment.
Allows one to basically use formulas to do things (pandas conquers excel!!!!)
You can do this (this could maybe have a bit better syntax though)