diff options
author | Barney Gale <barney.gale@gmail.com> | 2024-08-26 17:05:34 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-08-26 17:05:34 +0100 |
commit | 7bd6ebf696efcd5cf8e4e7946f9d8d8aee05664c (patch) | |
tree | 32e8e1088b2018438394b8a6409b95887f572b60 | |
parent | 033d537cd4b8c12f2441f1c23960c2153122140a (diff) | |
download | cpython-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.rst | 14 | ||||
-rw-r--r-- | Lib/pathlib/_abc.py | 52 | ||||
-rw-r--r-- | Lib/test/test_pathlib/test_pathlib_abc.py | 63 |
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) |