classification
Title: documentation example of os.walk should be less destructive
Type: Stage: resolved
Components: Documentation Versions: Python 3.8
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: docs@python Nosy List: aeros, docs@python, jftuga, python-dev, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2020-04-02 20:34 by jftuga, last changed 2020-05-24 11:33 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 19313 closed python-dev, 2020-04-02 21:22
Messages (10)
msg365625 - (view) Author: John Taylor (jftuga) * Date: 2020-04-02 20:34
The example for os.walkdir should be less destructive.  It currently recursively removes all files and directories.  I will be submitting a PR on GitHub.
msg365627 - (view) Author: John Taylor (jftuga) * Date: 2020-04-02 21:30
https://github.com/python/cpython/pull/19313

I have just signed the CLA.
msg365646 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-04-02 23:13
The proposed replacement doesn't succeed in demonstrating why topdown=False is necessary.  Consider doing a rename instead of a deletion or print.
msg365696 - (view) Author: John Taylor (jftuga) * Date: 2020-04-03 12:54
I would prefer an example that does not actually modify the file system.  Is there any way this could be achieved, yet still demonstrate why topdown=False is necessary?
msg365726 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-04-03 21:48
One possibility is a gathering cumulative directory statistics that include totals from all descendants (i.e. how many bytes of files would you save by removing the directory with rm -rf).

Outside of aggregating statistics, the normal reason to use topdown=False is when paths are being mutated.
msg365729 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-04-03 22:00
I do not think there is clearer example of topdown=False than recursive remove.

If you think that this example is destructive, consider how destructive is any possible example for shutil.rmtree()!
msg365735 - (view) Author: Kyle Stanley (aeros) * (Python committer) Date: 2020-04-04 01:16
Serhiy Storchaka wrote:
> I do not think there is clearer example of topdown=False than recursive remove.
>
> If you think that this example is destructive, consider how destructive is any possible example for shutil.rmtree()!

I concur with Serhiy. If the example is changed to just print out the file and directory path, as the PR proposes to do, it seems to defeat the purpose of using `topdown=False` (and the code example) in the first place.

If anything, I would suggest adding succinct comments or a note that very briefly explains how one could see a visual demonstration of what it does without removing any files or directories. For example: ``print(f"os.remove({os.path.join(root, name)})")``.
msg365736 - (view) Author: John Taylor (jftuga) * Date: 2020-04-04 02:30
I made the suggested change to just print the os.remove() statements (instead of executing them) and also removed the 'skip news'.
msg369765 - (view) Author: Kyle Stanley (aeros) * (Python committer) Date: 2020-05-24 02:26
> I made the suggested change to just print the os.remove() statements (instead of executing them) and also removed the 'skip news'.

I think you may have misunderstood the suggestion. 

Specifically, the key part was "I would suggest adding succinct comments or a note that very briefly explains how one could see a visual demonstration...". This would mean the actual code in the example would be *unchanged*, but with a new code comment or separate note after the example that explains how one could replace ``os.remove(os.path.join(root, name))`` with ``print(f"os.remove({os.path.join(root, name)})")`` for a purely visual demonstration that doesn't affect any local files.
msg369791 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-05-24 11:33
1. There is already a warning before example.
2. Even if you blindly copy-paste the example it will not work. You have to set the top variable.

So I don't see any problem here. You always can shoot yourself in the foot if try enough.
History
Date User Action Args
2020-05-24 11:33:10serhiy.storchakasetstatus: open -> closed
resolution: not a bug
messages: + msg369791

stage: patch review -> resolved
2020-05-24 02:26:37aerossetmessages: + msg369765
2020-04-04 02:30:51jftugasetmessages: + msg365736
2020-04-04 01:16:46aerossetnosy: + aeros
messages: + msg365735
2020-04-03 22:00:20serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg365729
2020-04-03 21:48:51rhettingersetmessages: + msg365726
2020-04-03 12:54:50jftugasetmessages: + msg365696
2020-04-02 23:13:45rhettingersetnosy: + rhettinger
messages: + msg365646
2020-04-02 21:30:54jftugasetmessages: + msg365627
2020-04-02 21:22:30python-devsetkeywords: + patch
nosy: + python-dev

pull_requests: + pull_request18678
stage: patch review
2020-04-02 20:34:49jftugacreate