-
-
Notifications
You must be signed in to change notification settings - Fork 33.6k
gh-137818: Add multiple if statements example for list comprehensions
#138217
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
base: main
Are you sure you want to change the base?
Conversation
sergey-miryanov
left a comment
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.
It looks valid for me.
Doc/tutorial/datastructures.rst
Outdated
| >>> [x for x in range(10) if x % 2 and x % 3] | ||
| [1, 5, 7] | ||
|
|
||
| Note the second :keyword:`!if` is replaced by :keyword:`!and`. |
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.
| Note the second :keyword:`!if` is replaced by :keyword:`!and`. | |
| Note that the second :keyword:`!if` has been replaced with :keyword:`!and`. |
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.
Adding 'that' would be correct if we add the note and patch, but I am considering whether to do so.
|
Looks redundant/misleading to me. As I said in the issue, the conditions are not and-ed, the clauses are nested and the documentation already says and shows that. |
Doc/tutorial/datastructures.rst
Outdated
| Note how the order of the :keyword:`for` and :keyword:`if` statements is the | ||
| same in both these snippets. | ||
|
|
||
| For multiple :keyword:`!if` statements, like 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.
Those aren't if statements but if clauses.
For example for a case of four clauses for-if-for-if, I can imagine the "and explanation" to confuse learners into thinking that those two ifs get and-ed somehow. Whereas thinking correctly about it, with nested statements as already documented, there's no issue. |
serhiy-storchaka
left a comment
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 do not think this is needed. This is trivial, and it is the same as for nested ifs. There is no reason to use two consequent if clauses, you can simply use and. No need to document an anti-pattern.
Framing this way I tend to change my mind. It'll be better to close this PR. |
Thanks to everyone for the reviews! It was also very helpful for my understanding. I will revoke my approval.
There's no need in this change at all
Not quite true, I have a reason and sometimes do that. When I split a "long" condition into multiple physical lines, I prefer one clause per line and having sub-conditions aligned vertically: lst = [
...
for x in ...
if longCondition
if anotherLongCondition
]I wouldn't like an |
|
Well, there may be stylistic reason. But it is not applicable for simple conditions like in the proposed example, and discussing stylistic nuances is out of scope of the language reference. |
willingc
left a comment
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.
Thanks for the submission.
| A list comprehension consists of brackets containing an expression followed | ||
| by a :keyword:`!for` clause, then zero or more :keyword:`!for` or :keyword:`!if` | ||
| clauses. The result will be a new list resulting from evaluating the expression | ||
| in the context of the :keyword:`!for` and :keyword:`!if` clauses which follow it. | ||
| For example, this listcomp combines the elements of two lists if they are not | ||
| equal:: |
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.
| A list comprehension consists of brackets containing an expression followed | |
| by a :keyword:`!for` clause, which may be followed by zero or more :keyword:`!for` or :keyword:`!if` | |
| clauses. The result will be a new list resulting from evaluating the expression | |
| in the context of the :keyword:`!for` and :keyword:`!if` clauses which follow it. | |
| For example, this listcomp combines the elements of two lists if they are not | |
| equal:: |
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 would make this change as I think the current wording created the initial confusion in the issue.
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.
Thank you for your help. My English is not very good, please forgive me.
Co-authored-by: Carol Willing <[email protected]>
📚 Documentation preview 📚: https://cpython-previews--138217.org.readthedocs.build/