ENH/API: Change query/eval local variable API

[cpcloud]

Currently, with query and eval you can use local variables a la the @ symbol. It's a bit confusing since you're not allowed to have a local variable and a column name with the same name, but it will try to pull the local if possible.

Current API:

Fails with a NameError:

a = 1
df  = DataFrame({'a': randn(10), 'b': randn(10)})
df.query('a > b')

But this works:

df.query('@a > b')

And so does this, which is confusing:

a = 1
df = DataFrame({'b': randn(10), 'c': randn(10)})
df.query('a < b < c')

As suggested by @y-p and @jreback, the following API is less confusing IMO.

From now on, all local variables will need an explicit reference and if there is a column name and a local with the same name then the column will be used. Thus you can always be sure that you're referring to a column, or it doesn't exist, in which case you'll get an error. And if you use @ then you can be sure that you're referring to local, and likewise get an error if it doesn't exist. As a bonus ( 🐺 in 🐑 's clothing), this allows you to use both a local and a column name with the same name.

Examples:

a = 1
df = DataFrame({'a': randn(10), 'b': randn(10)})

# uses the column 'a'
df.query('a > b')

# uses the local
df.query('@a > b')

# fails because I didn't reference the local and there's no 'c' column
c = 1
df.query('a > c')

# local and a column name
df.query('b < @a < a')

+1 for adopting "say what you mean" as a guiding principle.

[jreback]

+1 for cool emoji!

[cpcloud]

Slight update here: We'll have to keep the current scheme because otherwise locals in pd.eval would have to also have an @ prefix (essentially all vars would need @). I'll submit the PR as soon as it passes travis so folks can take a look

[cpcloud]

That means that

c = 1
df.query('a > c')

will still work because first frame cols are searched then locals, then globals. If they weren't, then toplevel pd.eval would require all vars to have a @ prefix.

However, you'll get a warning with a new parameter conflict='warn' and an exception with conflict='error'

[cpcloud]

I kind of feel like adding a keyword for this might betray a poor design decision on my part and this might require some more deliberation and discussion.

I agree re the conflict keyword, it's just throwing the problem at the user, not solving it.
If you think about the semantics a little differently, you could make the case that @ should
mean different things in df context and top level context.

Instead of saying @ means vars and no prefix means columns, think 'scope'.
no prefix means referncing local-scope, while referencing external-scope should
require an explicit @.

• in df context, i.e. df.eval() the scope consists of the df and variables are external.
• in top level context, i.e pd.eval(), locals are the scope and globals are external and should require @.
  You can (and should) modify this to mean variables in (local, global) resolution order are the scope
  and @ is an error that issues a warning to help orient users.

When you asked me to review PR a few months ago, I mentioned this ambiguity
problem as a smell, I still think it should go.

[cpcloud]

@y-p Agreed. Thanks.

[jreback]

doing this for 0.13.1?

[cpcloud]

Assuming that's in a few days...don't think so. This is going to take me a bit ... Just need to sit down with it for a while and hash it out in my head.

[jreback]

figured that