GeezerVison is showing github.com — GH-73991: Prune `pathlib.Path.copy()` and `copy_into()` arguments by barneygale · Pull Request #123337 · python/cpython
Conversation
barneygale
commented
Aug 26, 2024
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Remove ignore and on_error arguments from pathlib.Path.copy[_into](), because these arguments are under-designed. Specifically:
- ignore is appropriated from
shutil.copytree(), but it's not clear how it should apply when the user copies a non-directory. We've changed the callback signature from the shutil version, but I'm not confident the new signature is as good as it can be.
- on_error is a generalisation of
shutil.copytree()'s error handling, which is to accumulate exceptions and raise a single shutil.Error at the end. It's not obvious which solution is better.
Additionally, these arguments may be challenging to implement in future user subclasses of PathBase, which might utilise a native recursive copying method.
No news because copy() and copy_into() are unreleased.
📚 Documentation preview 📚: https://cpython-previews--123337.org.readthedocs.build/
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Remove *ignore* and *on_error* arguments from `pathlib.Path.copy[_into]()`,
because these arguments are under-designed. Specifically:
- *ignore* is appropriated from `shutil.copytree()`, but it's not clear
how it should apply when the user copies a non-directory. We've changed
the callback signature from the `shutil` version, but I'm not confident
the new signature is as good as it can be.
- *on_error* is a generalisation of `shutil.copytree()`'s error handling,
which is to accumulate exceptions and raise a single `shutil.Error` at
the end. It's not obvious which solution is better.
Additionally, this arguments may be challenging to implement in future user
subclasses of `PathBase`, which might utilise a native recursive copying
method.
|
I do think we need some replacement for the removed arguments, but unlike delete, I think it's more about enabling additional use cases and less about making sure that basic usage is OK. Specifically, I think ignore is important for filtered copies (for example, copy a project directory but don't copy generated files like *.pyc or *.obj). I'm less sure about on_error - mostly it's the same arguments as for delete, but in this case I don't know of any commonly-encountered cases that would need to be supported in day to day use.
So +1 from me on doing this as a temporary limitation, to get a minimal working implementation released.
|
barneygale reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks very much for the review! I agree with everything you wrote heh. I'll look to add support for filtering and customizing error handling soon. We might get a user request for them before too long!
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
pfmoore
pfmoore approved these changes
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|