Doc: Improve grammar in os.walk example#138466
Conversation
Comma should be a colon, as written at the same spot under `fwalk`.
|
I don’t see any particular benefit, it seems like a cosmetic change to me. |
How not? There's a comma splice. Replacing the comma with a colon fixes the grammatical error. |
|
|
||
| In the next example (simple implementation of :func:`shutil.rmtree`), | ||
| walking the tree bottom-up is essential, :func:`rmdir` doesn't allow | ||
| walking the tree bottom-up is essential: :func:`rmdir` doesn't allow |
There was a problem hiding this comment.
?
| walking the tree bottom-up is essential: :func:`rmdir` doesn't allow | |
| walking the tree bottom-up is essential, because :func:`rmdir` doesn't allow |
There was a problem hiding this comment.
Sure, that's also a good suggestion, but fwalk already uses a colon:
Lines 3767 to 3768 in 424e2ab
So I used the same for consistency. No need to edit two spots when one will do, but if you think it would be clearer to add "because", by all means.
Another option is:
walking the tree bottom-up is essential since :func:`rmdir` doesn't allow
There was a problem hiding this comment.
We could also use a semicolon, but I like this more.
There was a problem hiding this comment.
@ZeroIntensity Yeah, a semicolon would fix the grammar, but using a colon does the same and implies "because".
There was a problem hiding this comment.
Excess brevity is not called for here, I think 'because' is fine. Given it's immediately before a role that also starts with :, having the colon could be a little confusing.
There was a problem hiding this comment.
No need to edit two spots when one will do, but if you think it would be clearer to add "because", by all means.
It's fine to have inconsistent styles -- English doesn't follow PEP 8 :)
|
I've removed |
|
Please do not use the Update Branch button unless necessary (e.g. fixing conflicts, jogging the CI, or very old PRs) as it uses valuable resources. For more information see the devguide. |
|
@StanFromIreland Yeah, sorry, I'm so used to clicking it mindlessly for something else I'm working on then realized it was totally unnecessary here. |
Comma should be a colon, as written at the same spot under
fwalk.📚 Documentation preview 📚: https://cpython-previews--138466.org.readthedocs.build/
Specifically: https://cpython-previews--138466.org.readthedocs.build/en/138466/library/os.html#os.walk