-
Notifications
You must be signed in to change notification settings - Fork 41
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
List comprehension with multiple for
statements does not evaluate correctly
#127
Comments
for
s does not evaluate correctly
for
s does not evaluate correctlyfor
statements does not evaluate correctly
Somewhat related to #126 |
@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 ;)). |
Thanks for the response.
Yeah, I just connected this with #126 because of the linked commits e2b64e9 dcb1aa3, which added test cases with multiple "for"s.
Do you mean visually or programmatically? 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. Footnotes
|
By "hard to read", I would say "visually and programmatically".
Here, So, translating
should translate to |
Good point... I agree it is counter-intuitive. So I'll have to take a step back on "straightforward" here... 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 |
@mara004 this might be now fixed -- all tests (including your example) should now pass. +1 on the rant about |
Thanks for the quick fix! I can confirm the above example now passes. |
In case you're interested, this is where I'm using |
@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! |
MRE:
Tested with latest release (
0.9.33
) and master (b9a9b64) as of this writing.The text was updated successfully, but these errors were encountered: