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

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.
This commit is contained in:
Barney Gale 2024-08-26 17:05:34 +01:00 committed by GitHub
parent 033d537cd4
commit 7bd6ebf696
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 21 additions and 108 deletions

View file

@ -1540,7 +1540,7 @@ Copying, moving and deleting
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
.. method:: Path.copy(target, *, follow_symlinks=True, dirs_exist_ok=False, \ .. method:: Path.copy(target, *, follow_symlinks=True, dirs_exist_ok=False, \
preserve_metadata=False, ignore=None, on_error=None) preserve_metadata=False)
Copy this file or directory tree to the given *target*, and return a new Copy this file or directory tree to the given *target*, and return a new
:class:`!Path` instance pointing to *target*. :class:`!Path` instance pointing to *target*.
@ -1563,21 +1563,11 @@ Copying, moving and deleting
This argument has no effect when copying files on Windows (where This argument has no effect when copying files on Windows (where
metadata is always preserved). metadata is always preserved).
If *ignore* is given, it should be a callable accepting one argument: a
source file or directory path. The callable may return true to suppress
copying of the path.
If *on_error* is given, it should be a callable accepting one argument: an
instance of :exc:`OSError`. The callable may re-raise the exception or do
nothing, in which case the copying operation continues. If *on_error* isn't
given, exceptions are propagated to the caller.
.. versionadded:: 3.14 .. versionadded:: 3.14
.. method:: Path.copy_into(target_dir, *, follow_symlinks=True, \ .. method:: Path.copy_into(target_dir, *, follow_symlinks=True, \
dirs_exist_ok=False, preserve_metadata=False, \ dirs_exist_ok=False, preserve_metadata=False)
ignore=None, on_error=None)
Copy this file or directory tree into the given *target_dir*, which should Copy this file or directory tree into the given *target_dir*, which should
be an existing directory. Other arguments are handled identically to be an existing directory. Other arguments are handled identically to

View file

@ -865,23 +865,16 @@ class PathBase(PurePathBase):
raise raise
def copy(self, target, *, follow_symlinks=True, dirs_exist_ok=False, def copy(self, target, *, follow_symlinks=True, dirs_exist_ok=False,
preserve_metadata=False, ignore=None, on_error=None): preserve_metadata=False):
""" """
Recursively copy this file or directory tree to the given destination. Recursively copy this file or directory tree to the given destination.
""" """
if not isinstance(target, PathBase): if not isinstance(target, PathBase):
target = self.with_segments(target) target = self.with_segments(target)
try:
self._ensure_distinct_path(target) self._ensure_distinct_path(target)
except OSError as err:
if on_error is None:
raise
on_error(err)
return
stack = [(self, target)] stack = [(self, target)]
while stack: while stack:
src, dst = stack.pop() src, dst = stack.pop()
try:
if not follow_symlinks and src.is_symlink(): if not follow_symlinks and src.is_symlink():
dst._symlink_to_target_of(src) dst._symlink_to_target_of(src)
if preserve_metadata: if preserve_metadata:
@ -889,24 +882,18 @@ class PathBase(PurePathBase):
elif src.is_dir(): elif src.is_dir():
children = src.iterdir() children = src.iterdir()
dst.mkdir(exist_ok=dirs_exist_ok) dst.mkdir(exist_ok=dirs_exist_ok)
for child in children: stack.extend((child, dst.joinpath(child.name))
if not (ignore and ignore(child)): for child in children)
stack.append((child, dst.joinpath(child.name)))
if preserve_metadata: if preserve_metadata:
src._copy_metadata(dst) src._copy_metadata(dst)
else: else:
src._copy_file(dst) src._copy_file(dst)
if preserve_metadata: if preserve_metadata:
src._copy_metadata(dst) src._copy_metadata(dst)
except OSError as err:
if on_error is None:
raise
on_error(err)
return target return target
def copy_into(self, target_dir, *, follow_symlinks=True, def copy_into(self, target_dir, *, follow_symlinks=True,
dirs_exist_ok=False, preserve_metadata=False, ignore=None, dirs_exist_ok=False, preserve_metadata=False):
on_error=None):
""" """
Copy this file or directory tree into the given existing directory. Copy this file or directory tree into the given existing directory.
""" """
@ -919,8 +906,7 @@ class PathBase(PurePathBase):
target = self.with_segments(target_dir, name) target = self.with_segments(target_dir, name)
return self.copy(target, follow_symlinks=follow_symlinks, return self.copy(target, follow_symlinks=follow_symlinks,
dirs_exist_ok=dirs_exist_ok, dirs_exist_ok=dirs_exist_ok,
preserve_metadata=preserve_metadata, ignore=ignore, preserve_metadata=preserve_metadata)
on_error=on_error)
def rename(self, target): def rename(self, target):
""" """

View file

@ -1984,14 +1984,6 @@ class DummyPathTest(DummyPurePathTest):
self.assertRaises(OSError, source.copy, source) self.assertRaises(OSError, source.copy, source)
self.assertRaises(OSError, source.copy, source, follow_symlinks=False) self.assertRaises(OSError, source.copy, source, follow_symlinks=False)
def test_copy_dir_to_itself_on_error(self):
base = self.cls(self.base)
source = base / 'dirC'
errors = []
source.copy(source, on_error=errors.append)
self.assertEqual(len(errors), 1)
self.assertIsInstance(errors[0], OSError)
def test_copy_dir_into_itself(self): def test_copy_dir_into_itself(self):
base = self.cls(self.base) base = self.cls(self.base)
source = base / 'dirC' source = base / 'dirC'
@ -2000,61 +1992,6 @@ class DummyPathTest(DummyPurePathTest):
self.assertRaises(OSError, source.copy, target, follow_symlinks=False) self.assertRaises(OSError, source.copy, target, follow_symlinks=False)
self.assertFalse(target.exists()) self.assertFalse(target.exists())
def test_copy_missing_on_error(self):
base = self.cls(self.base)
source = base / 'foo'
target = base / 'copyA'
errors = []
result = source.copy(target, on_error=errors.append)
self.assertEqual(result, target)
self.assertEqual(len(errors), 1)
self.assertIsInstance(errors[0], FileNotFoundError)
def test_copy_dir_ignore_false(self):
base = self.cls(self.base)
source = base / 'dirC'
target = base / 'copyC'
ignores = []
def ignore_false(path):
ignores.append(path)
return False
result = source.copy(target, ignore=ignore_false)
self.assertEqual(result, target)
self.assertEqual(set(ignores), {
source / 'dirD',
source / 'dirD' / 'fileD',
source / 'fileC',
source / 'novel.txt',
})
self.assertTrue(target.is_dir())
self.assertTrue(target.joinpath('dirD').is_dir())
self.assertTrue(target.joinpath('dirD', 'fileD').is_file())
self.assertEqual(target.joinpath('dirD', 'fileD').read_text(),
"this is file D\n")
self.assertTrue(target.joinpath('fileC').is_file())
self.assertTrue(target.joinpath('fileC').read_text(),
"this is file C\n")
def test_copy_dir_ignore_true(self):
base = self.cls(self.base)
source = base / 'dirC'
target = base / 'copyC'
ignores = []
def ignore_true(path):
ignores.append(path)
return True
result = source.copy(target, ignore=ignore_true)
self.assertEqual(result, target)
self.assertEqual(set(ignores), {
source / 'dirD',
source / 'fileC',
source / 'novel.txt',
})
self.assertTrue(target.is_dir())
self.assertFalse(target.joinpath('dirD').exists())
self.assertFalse(target.joinpath('fileC').exists())
self.assertFalse(target.joinpath('novel.txt').exists())
@needs_symlinks @needs_symlinks
def test_copy_dangling_symlink(self): def test_copy_dangling_symlink(self):
base = self.cls(self.base) base = self.cls(self.base)