bpo-44347: [doc] clarify meaning of shutil.copytree's dirs_exist_ok kwarg by jdevries3133 · Pull Request #26643 · python/cpython

jdevries3133 · 2021-06-10T12:29:32Z

It now mirrors the documentation for os.makedirs
Docstring in python source file was correspondingly updated

https://bugs.python.org/issue44347

…kwarg * It now mirrors the documentation for os.makedirs * Docstring in python source file was correspondingly updated

* Add additional clarification: nonexistent parent directories will also be created.

jdevries3133 · 2021-06-10T13:57:22Z

@tvogel, continuing our conversation from now-closed #26634...

I agree, that should be mentioned and I added more detail for that. Additionally, I moved everything about dirs_exist_ok down to the bottom of the documentation and the docstring to match the order of arguments in the function signature.

Key Sentence Added

Furthermore, any parent directory in the path to dst will be created if they do not exist when the dirs_exist_ok flag is True.

nanjekyejoannah

Thanks for your contribution @jdevries3133, can you please fix the errors reported in the CI?

jdevries3133 · 2021-06-10T21:06:22Z

@nanjekyejoannah I fixed it. Thank you for your patience!

akulakov · 2021-06-11T05:37:00Z

+   that supports the same signature (like :func:`~shutil.copy`) can be used.
+
+   If *dirs_exist_ok* is false (the default), a :exc:`FileExistsError` is
+   raised if *dst* already exists. Conversely, *dst* as well as any any parent


I think this phrasing is a bit unclear. 'conversely' implies that the following is opposite to how the default value works. In other words, I would read this sentence as saying that with dirs_exist_ok=False, dirs along the path to dst, as well as dst itself, will not be created.

Yeah, you are totally correct; I've made it more confusing. I changed it to be much more explicit.

…ok=False) * Be more explicit

jdevries3133 · 2021-06-11T21:11:41Z

I think the failing test is related to bpo-38323. I only made changes to docs / docstrings, and make check html SPHINXOPTS="-q -W -j4" was happy with me.

…onGH-26645)

In particular, when running with tk8.6.8, as in PSF 3.9. Co-authored-by: Terry Jan Reedy <[email protected]>

…-26606) Change the behaviour of `math.pow(0.0, -math.inf)` and `math.pow(-0.0, -math.inf)` to return positive infinity instead of raising `ValueError`. This makes `math.pow` consistent with the built-in `pow` (and the `**` operator) for this particular special case, and brings the `math.pow` special-case handling into compliance with IEEE 754.

…onGH-26679)

Greatly reduce pow() overhead for small exponents.

…er buffers (pythonGH-26676) Automerge-Triggered-By: GH:pablogsal

…6693)

Co-authored-by: Terry Jan Reedy <[email protected]>

Signed-off-by: Christian Heimes <[email protected]>

…results (pythonGH-26715) * Simplify the count_vowels example * Hits and misses are fetched while a lock is held * Add note that references are kept for arguments and return values * Clarify behavior when *typed* is false.

…-26638) * Add specializations of LOAD_GLOBAL. * Add more stats. * Remove old opcache; it is no longer used. * Add NEWS

jdevries3133 · 2021-06-21T17:14:01Z

@akulakov Ah, I see what you are saying; however, I'm still happy with what I added because:

dirs_exist_ok is a good name for the parameter here; simply exist_ok would not be a better name. In os.makedirs(exist_ok=False), it is clear to the user that this switch only applies to directories (because makedirs itself obviously only touches directories). shutil.copytree's function name itself does not provide the same context, though, so it is worthwhile for the parameter to be more explicit.
Referencing os.makedirs will only help users who already know how that function behaves. For newcomers, I don't want to give only a breadcrumb and send them on a scavenger hunt.

All that being said, I did add one sentence about the relationship between dirs_exist_ok and os.makedirs's exist_ok parameters.

github-actions · 2021-08-01T00:04:59Z

This PR is stale because it has been open for 30 days with no activity.

* remove the note which is excessive * remove all changes to `Lib/shutil.py`, pending approval of the new docs, I'll re-add revisions to shutil.copytree's docstring. * revise first paragraph

jdevries3133 · 2022-04-08T22:02:43Z

@JelleZijlstra I revised the PR. If it looks good, I will add the paragraph about dirs_exist_ok to the docstring in Lib/shutil.py, and then this should be good to go.

I deleted 80% of the PR and rewrote the rest, so please review as if this is all new.

JelleZijlstra

Thanks!

JelleZijlstra

The text looks good! Can you update the docstring too and undraft the PR?

ghost · 2022-04-10T15:30:50Z

Commit authors are required to sign the Contributor License Agreement.

jdevries3133 · 2022-04-10T15:33:59Z

@JelleZijlstra OK, good to go. Not sure why the CLA bot is complaining, I clicked the link and re-signed but it still says I'm unsigned. I'm guessing it's just a GitHub migration glitch, I've made a handful of contributions in the past and did sign the CLA.

jdevries3133 · 2022-04-10T15:34:46Z

CI failure is a fluke:

E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/c/ccache/ccache_3.7.7-1_amd64.deb  Connection failed [IP: 52.147.219.192 80]
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?
Error: Process completed with exit code 100.

Can you trigger a re-run?

AlexWaygood · 2022-04-10T15:39:03Z

CI failure is a fluke:

E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/c/ccache/ccache_3.7.7-1_amd64.deb  Connection failed [IP: 52.147.219.192 80]
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?
Error: Process completed with exit code 100.

Can you trigger a re-run?

Probably related to python/the-knights-who-say-ni#318

JelleZijlstra · 2022-04-10T21:00:06Z

I wonder if the CLA bot is unhappy because of all the random commits that were added to the branch earlier.

AlexWaygood · 2022-04-10T21:02:35Z

I wonder if the CLA bot is unhappy because of all the random commits that were added to the branch earlier

Perhaps a force-push would sort that out?

jdevries3133 · 2022-04-11T00:36:23Z

I just created a new branch and a new PR with a single commit:

#91434

Use it if that's helpful, close it if not :)

JelleZijlstra · 2022-04-12T00:58:17Z

Merged in #91434.

bpo-44347: [doc] clarify meaning of shutil.copytree's dirs_exist_ok …

74727b9

…kwarg * It now mirrors the documentation for os.makedirs * Docstring in python source file was correspondingly updated

the-knights-who-say-ni added the CLA signed label Jun 10, 2021

bedevere-bot added the awaiting review label Jun 10, 2021

jdevries3133 added 2 commits June 10, 2021 09:40

bpo-44347: [doc] clarify shutil.copytree dirs_exist_ok kwarg

0fa96d1

* Add additional clarification: nonexistent parent directories will also be created.

Merge branch 'main' of github.com:python/cpython into bpo-44347

318a99c

nanjekyejoannah reviewed Jun 10, 2021

View reviewed changes

jdevries3133 added 2 commits June 10, 2021 17:01

bpo-44347: [doc|fix]: add NEWS.d entry, remove use of default context

9b262b8

Merge branch 'main' of github.com:python/cpython into bpo-44347

952dbf1

akulakov reviewed Jun 11, 2021

View reviewed changes

FFY00 added the docs Documentation in the Doc dir label Jun 11, 2021

jdevries3133 added 3 commits June 11, 2021 15:28

bpo-44347: [doc] better document argument shutil.copytree(dirs_exist_…

e365e52

…ok=False) * Be more explicit

Merge branch 'main' of github.com:python/cpython into bpo-44347

eb3ee6f

bpo-44347: [doc|fix]: add missed updated docstring from previous commit

d340c87

zooba and others added 14 commits June 19, 2021 11:00

bpo-44381: Windows build now allows enabling control flow guard (pyth…

dd2f5c3

…onGH-26645)

bpo-40128: Fix IDLE autocomplete on macOS (pythonGH-26672)

d8837d8

In particular, when running with tk8.6.8, as in PSF 3.9. Co-authored-by: Terry Jan Reedy <[email protected]>

bpo-43475: Fix the Python implementation of hash of Decimal NaN (pyth…

29584b7

…onGH-26679)

Add more const modifiers. (pythonGH-26691)

b63a793

bpo-44376 - reduce pow() overhead for small exponents (pythonGH-26662)

feca673

Greatly reduce pow() overhead for small exponents.

bpo-44396: Update multi-line-start location when reallocating tokeniz…

b1e8a39

…er buffers (pythonGH-26676) Automerge-Triggered-By: GH:pablogsal

bpo-44389: Remove duplicate SSL_OP_NO_TLSv1_2 flag (pythonGH-26680)

732ecab

bpo-43425: Update test_c_parser not to use TempdirManager (pythonGH-2…

2a93215

…6693)

Fix typos in multiple files (pythonGH-26689)

50aee65

Co-authored-by: Terry Jan Reedy <[email protected]>

Fix a potential reference-counting bug in long_pow (pythonGH-26690)

6966890

bpo-44389: Fix deprecation of OP_NO_TLSv1_3 (pythonGH-26700)

d1a2233

Signed-off-by: Christian Heimes <[email protected]>

bpo-44338: Port LOAD_GLOBAL to PEP 659 adaptive interpreter (pythonGH…

0208b06

…-26638) * Add specializations of LOAD_GLOBAL. * Add more stats. * Remove old opcache; it is no longer used. * Add NEWS

bpo-44347: highlight relationship to os.makedirs

48f30ee

small rewording fix

5b8366a

github-actions Bot added the stale Stale PR or inactive for long period of time. label Aug 1, 2021

JelleZijlstra reviewed Apr 2, 2022

View reviewed changes

Comment thread Doc/library/shutil.rst Outdated

jdevries3133 marked this pull request as draft April 3, 2022 00:22

jdevries3133 added 2 commits April 8, 2022 17:16

Merge branch 'main' of github.com:python/cpython into bpo-44347

60a0eb6

revise old changes

8303dc5

* remove the note which is excessive * remove all changes to `Lib/shutil.py`, pending approval of the new docs, I'll re-add revisions to shutil.copytree's docstring. * revise first paragraph

JelleZijlstra self-requested a review April 8, 2022 22:10

JelleZijlstra reviewed Apr 9, 2022

View reviewed changes

Comment thread Doc/library/shutil.rst Outdated

Comment thread Doc/library/shutil.rst Outdated

apply @JelleZijlstra's feedback

c169b1d

JelleZijlstra reviewed Apr 10, 2022

View reviewed changes

update docstring

ac3f837

jdevries3133 marked this pull request as ready for review April 10, 2022 15:31

JelleZijlstra approved these changes Apr 10, 2022

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Apr 10, 2022

jdevries3133 mentioned this pull request Apr 11, 2022

gh-88513: [DOC] clarify shutil.copytree's dirs_exist_ok arg #91434

Merged

JelleZijlstra closed this Apr 12, 2022

tilmanvogel mannequin mentioned this pull request Apr 12, 2022

Unclear documentation for shutil.copytree() #88513

Closed

Uh oh!

Conversation

jdevries3133 commented Jun 10, 2021 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdevries3133 commented Jun 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Sentence Added

Uh oh!

nanjekyejoannah left a comment

Choose a reason for hiding this comment

Uh oh!

jdevries3133 commented Jun 10, 2021

Uh oh!

akulakov Jun 11, 2021

Choose a reason for hiding this comment

Uh oh!

jdevries3133 Jun 11, 2021

Choose a reason for hiding this comment

Uh oh!

jdevries3133 commented Jun 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdevries3133 commented Jun 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 1, 2021

Uh oh!

Uh oh!

jdevries3133 commented Apr 8, 2022

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Apr 10, 2022

Uh oh!

jdevries3133 commented Apr 10, 2022

Uh oh!

jdevries3133 commented Apr 10, 2022

Uh oh!

AlexWaygood commented Apr 10, 2022

Uh oh!

JelleZijlstra commented Apr 10, 2022

Uh oh!

AlexWaygood commented Apr 10, 2022

Uh oh!

jdevries3133 commented Apr 11, 2022

Uh oh!

JelleZijlstra commented Apr 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

jdevries3133 commented Jun 10, 2021 •

edited by bedevere-bot

Loading

jdevries3133 commented Jun 10, 2021 •

edited

Loading

jdevries3133 commented Jun 11, 2021 •

edited

Loading

jdevries3133 commented Jun 21, 2021 •

edited

Loading