saulpw / visidata

A terminal spreadsheet multitool for discovering and arranging data
http://visidata.org
GNU General Public License v3.0
7.91k stars 283 forks source link

[eval] can't use locals in list comprehensions #1627

Closed librarianmage closed 1 year ago

librarianmage commented 1 year ago

Small description

When using list comprehensions over expr columns that return lists, columns that are not lists end up returning NameErrors

Expected result

The column should be the same for each iteration over the loop, because it is not a list

Actual result with screenshot

image

Steps to reproduce with sample data and a .vd Please attach the commandlog (saved with Ctrl-D) to show the steps that led to the issue. See here for more details. customers.vdj.txt

Additional context Please include the version of VisiData. saul.pw/VisiData v2.10.2

saulpw commented 1 year ago

Hi @librarianmage, this is actually an unfortunate issue with Python eval() and using local variables in list comprehensions:

>>> expr = '[x+i for x in foo]'
>>> print(eval(expr, dict(foo=[1,2,3], i=7), {}))
[8, 9, 10]
>>> print(eval(expr, dict(i=7), dict(foo=[1,2,3])))
[8, 9, 10]
>>> print(eval(expr, dict(foo=[1,2,3]), dict(i=7)))                                                                                                                                                                                                                
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 1, in <module>
  File "<string>", line 1, in <listcomp>
NameError: name 'i' is not defined. Did you mean: 'id'?

From the docs on Python eval:

If provided, globals must be a dictionary.

Note, eval() does not have access to the nested scopes (non-locals) in the enclosing environment.

So, VisiData's evalExpr has to pass the mapping of column names to the cell values as locals, because it's expensive to compute all column values and attributes every time; instead it computes the value of a column or attribute only when an expression actually uses it. This is 99% totally fine, but list comprehensions in Python create a new local scope, and within eval are not allowed to reach into the enclosing local scope.

I agree that this annoying and incorrect behavior, but I haven't found a way to make it work like it seems it should.