List comprehension with multiple `for` statements does not evaluate correctly

[mara004]

MRE:

odd = [1, 3, 5, 7, 9, 11, 13, 15, 17, 19]
even = [2, 4, 6, 8, 10, 12, 14, 16, 18, 20]

import asteval
myeval = asteval.Interpreter(user_symbols={"odd": odd, "even": even})
print( myeval("[n for p in zip(odd, even) for n in p]") )
# wrong: [19, 20, 19, 20, 19, 20, 19, 20, 19, 20, 19, 20, 19, 20, 19, 20, 19, 20, 19, 20]

print( [n for p in zip(odd, even) for n in p] )
# correct: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20]

Tested with latest release (0.9.33) and master (b9a9b64) as of this writing.

[mara004]

Somewhat related to https://github.com/lmfit/asteval/issues/126

[newville]

@mara004 Thanks - I can duplicate this error. FWIW, I think it probably is not that closely related to #126: I think it is not about the values, but about handling multiple "for" statements.

I might be willing to say that multiple list comprehension is so hard to read correctly that we could simply not support it. But, I will try to look at this (and also admit that it is not my highest priority ;)).

[mara004]

Thanks for the response.

FWIW, I think it probably is not that closely related to https://github.com/lmfit/asteval/issues/126: I think it is not about the values, but about handling multiple "for" statements.

Yeah, I just connected this with #126 because of the linked commits https://github.com/lmfit/asteval/commit/e2b64e9527e411a66627ac14f245d1081b38fae3 https://github.com/lmfit/asteval/commit/dcb1aa35e8b3a206a1b0d4786904be03f14596f5, which added test cases with multiple "for"s.

I might be willing to say that multiple list comprehension is so hard to read correctly that we could simply not support it.

Do you mean visually or programmatically? If visually, I find they're actually fairly straightforward to read once you think of the for statements as a tree and the part before the leftmost for as the content to append to the list in the innermost for.[^1]

IMHO listcomps with multiple "for"s are a pretty essential means of creating flat lists in an expression, so I'm not in favor of intentionally not supporting them. Anyway, thanks for planning to look into this; take your time.

[^1]: Illustration: https://stackoverflow.com/a/45079294/15547292

[newville]

@mara004

By "hard to read", I would say "visually and programmatically". Basicaally, a = [x*2 for x in range(10)] is a shortened, one-line version of

a = []
for x in range(10):
     a.append(x*2)

Here, a only uses append, because a list is being generated. That's fine. The assignment of "value to append" comes before the loop. OK.

So, translating

a = []
for p in zip(odd, even):
     for n in p:
         a.append(n)

should translate to a = [n for n in p for p in zip(odd, even)]. I would be able to understand that (and so more complicated variations), but a = [n for p in zip(odd, even) for n in p] is just awful. This is one of the few cases where I would call Python "pathologically inconsistent" and "horrible".

[mara004]

Good point... I agree it is counter-intuitive. So I'll have to take a step back on "straightforward" here... Given that the value to append comes first, you'd expect the loops to be specified from inner to outer as well. This mix of directions, value first but loops the other way, is indeed weird – that way, one may have to specify a variable name far before its defining loop.

Anyway, I'm afraid that's just how it is, and we have to arrange ourselves with that reality. I was able to get used to it, as this affects only order of code but not functionality.

Personally, I would reserve terms like "awful" to match/case, though (PEPs 634-636). Try matching against a non-dotted constant – it's ridiculous. 😖🤦‍♂️ Not to mention the inconsistent abuse of the | (binary or) operator, or the excessive indentation. Sorry for the off-topic rant.

[newville]

@mara004 this might be now fixed -- all tests (including your example) should now pass.

+1 on the rant about match/case ;)

[mara004]

Thanks for the quick fix! I can confirm the above example now passes.

[mara004]

In case you're interested, this is where I'm using asteval: https://gist.github.com/mara004/0ef12eaa154502f25de2aee94e82f84b/9e2b1ea71a740f2a1d2ae44ac33a41495a4035b1 I have no prior knowledge on parsing, though, so I have no idea whether what I'm doing is good or bad... It might be bloatware, but was interesting to write in any case ;)

[newville]

@mara004 OK, thanks -- I'm not sure I understand it (is the idea to parse different ways of expressing ranges of page numbers?), but great!