Skip to content
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

GH-73991: Prune pathlib.Path.copy() and copy_into() arguments #123337

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

barneygale
Copy link
Contributor

@barneygale barneygale commented Aug 26, 2024

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/

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.
@pfmoore
Copy link
Member

pfmoore commented Aug 26, 2024

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
Copy link
Contributor Author

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!

@barneygale barneygale merged commit 7bd6ebf into python:main Aug 26, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants