aboutsummaryrefslogtreecommitdiffstatshomepage
diff options
context:
space:
mode:
authorBarney Gale <barney.gale@gmail.com>2024-08-26 17:05:34 +0100
committerGitHub <noreply@github.com>2024-08-26 17:05:34 +0100
commit7bd6ebf696efcd5cf8e4e7946f9d8d8aee05664c (patch)
tree32e8e1088b2018438394b8a6409b95887f572b60
parent033d537cd4b8c12f2441f1c23960c2153122140a (diff)
downloadcpython-7bd6ebf696efcd5cf8e4e7946f9d8d8aee05664c.tar.gz
cpython-7bd6ebf696efcd5cf8e4e7946f9d8d8aee05664c.zip
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.
-rw-r--r--Doc/library/pathlib.rst14
-rw-r--r--Lib/pathlib/_abc.py52
-rw-r--r--Lib/test/test_pathlib/test_pathlib_abc.py63
3 files changed, 21 insertions, 108 deletions
diff --git a/Doc/library/pathlib.rst b/Doc/library/pathlib.rst
index 4a8de261770..1c54ceacbc7 100644
--- a/Doc/library/pathlib.rst
+++ b/Doc/library/pathlib.rst
@@ -1540,7 +1540,7 @@ Copying, moving and deleting
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
.. 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
: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
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
.. method:: Path.copy_into(target_dir, *, follow_symlinks=True, \
- dirs_exist_ok=False, preserve_metadata=False, \
- ignore=None, on_error=None)
+ dirs_exist_ok=False, preserve_metadata=False)
Copy this file or directory tree into the given *target_dir*, which should
be an existing directory. Other arguments are handled identically to
diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py
index 2c246430a69..11c8018b28f 100644
--- a/Lib/pathlib/_abc.py
+++ b/Lib/pathlib/_abc.py
@@ -865,48 +865,35 @@ class PathBase(PurePathBase):
raise
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.
"""
if not isinstance(target, PathBase):
target = self.with_segments(target)
- try:
- self._ensure_distinct_path(target)
- except OSError as err:
- if on_error is None:
- raise
- on_error(err)
- return
+ self._ensure_distinct_path(target)
stack = [(self, target)]
while stack:
src, dst = stack.pop()
- try:
- if not follow_symlinks and src.is_symlink():
- dst._symlink_to_target_of(src)
- if preserve_metadata:
- src._copy_metadata(dst, follow_symlinks=False)
- elif src.is_dir():
- children = src.iterdir()
- dst.mkdir(exist_ok=dirs_exist_ok)
- for child in children:
- if not (ignore and ignore(child)):
- stack.append((child, dst.joinpath(child.name)))
- if preserve_metadata:
- src._copy_metadata(dst)
- else:
- src._copy_file(dst)
- if preserve_metadata:
- src._copy_metadata(dst)
- except OSError as err:
- if on_error is None:
- raise
- on_error(err)
+ if not follow_symlinks and src.is_symlink():
+ dst._symlink_to_target_of(src)
+ if preserve_metadata:
+ src._copy_metadata(dst, follow_symlinks=False)
+ elif src.is_dir():
+ children = src.iterdir()
+ dst.mkdir(exist_ok=dirs_exist_ok)
+ stack.extend((child, dst.joinpath(child.name))
+ for child in children)
+ if preserve_metadata:
+ src._copy_metadata(dst)
+ else:
+ src._copy_file(dst)
+ if preserve_metadata:
+ src._copy_metadata(dst)
return target
def copy_into(self, target_dir, *, follow_symlinks=True,
- dirs_exist_ok=False, preserve_metadata=False, ignore=None,
- on_error=None):
+ dirs_exist_ok=False, preserve_metadata=False):
"""
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)
return self.copy(target, follow_symlinks=follow_symlinks,
dirs_exist_ok=dirs_exist_ok,
- preserve_metadata=preserve_metadata, ignore=ignore,
- on_error=on_error)
+ preserve_metadata=preserve_metadata)
def rename(self, target):
"""
diff --git a/Lib/test/test_pathlib/test_pathlib_abc.py b/Lib/test/test_pathlib/test_pathlib_abc.py
index 7cddc386d41..08355a71453 100644
--- a/Lib/test/test_pathlib/test_pathlib_abc.py
+++ b/Lib/test/test_pathlib/test_pathlib_abc.py
@@ -1984,14 +1984,6 @@ class DummyPathTest(DummyPurePathTest):
self.assertRaises(OSError, source.copy, source)
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):
base = self.cls(self.base)
source = base / 'dirC'
@@ -2000,61 +1992,6 @@ class DummyPathTest(DummyPurePathTest):
self.assertRaises(OSError, source.copy, target, follow_symlinks=False)
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
def test_copy_dangling_symlink(self):
base = self.cls(self.base)