Skip to content

Commit 8d25830

Browse files
committed
pythonGH-73991: Prune pathlib.Path.copy() and copy_into() arguments
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.
1 parent c68a93c commit 8d25830

File tree

4 files changed

+21
-113
lines changed

4 files changed

+21
-113
lines changed

Doc/library/pathlib.rst

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1540,7 +1540,7 @@ Copying, moving and deleting
15401540
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15411541

15421542
.. method:: Path.copy(target, *, follow_symlinks=True, dirs_exist_ok=False, \
1543-
preserve_metadata=False, ignore=None, on_error=None)
1543+
preserve_metadata=False)
15441544

15451545
Copy this file or directory tree to the given *target*, and return a new
15461546
:class:`!Path` instance pointing to *target*.
@@ -1563,21 +1563,11 @@ Copying, moving and deleting
15631563
This argument has no effect when copying files on Windows (where
15641564
metadata is always preserved).
15651565

1566-
If *ignore* is given, it should be a callable accepting one argument: a
1567-
source file or directory path. The callable may return true to suppress
1568-
copying of the path.
1569-
1570-
If *on_error* is given, it should be a callable accepting one argument: an
1571-
instance of :exc:`OSError`. The callable may re-raise the exception or do
1572-
nothing, in which case the copying operation continues. If *on_error* isn't
1573-
given, exceptions are propagated to the caller.
1574-
15751566
.. versionadded:: 3.14
15761567

15771568

15781569
.. method:: Path.copy_into(target_dir, *, follow_symlinks=True, \
1579-
dirs_exist_ok=False, preserve_metadata=False, \
1580-
ignore=None, on_error=None)
1570+
dirs_exist_ok=False, preserve_metadata=False)
15811571

15821572
Copy this file or directory tree into the given *target_dir*, which should
15831573
be an existing directory. Other arguments are handled identically to

Lib/pathlib/_abc.py

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -865,48 +865,35 @@ def _copy_file(self, target):
865865
raise
866866

867867
def copy(self, target, *, follow_symlinks=True, dirs_exist_ok=False,
868-
preserve_metadata=False, ignore=None, on_error=None):
868+
preserve_metadata=False):
869869
"""
870870
Recursively copy this file or directory tree to the given destination.
871871
"""
872872
if not isinstance(target, PathBase):
873873
target = self.with_segments(target)
874-
try:
875-
self._ensure_distinct_path(target)
876-
except OSError as err:
877-
if on_error is None:
878-
raise
879-
on_error(err)
880-
return
874+
self._ensure_distinct_path(target)
881875
stack = [(self, target)]
882876
while stack:
883877
src, dst = stack.pop()
884-
try:
885-
if not follow_symlinks and src.is_symlink():
886-
dst._symlink_to_target_of(src)
887-
if preserve_metadata:
888-
src._copy_metadata(dst, follow_symlinks=False)
889-
elif src.is_dir():
890-
children = src.iterdir()
891-
dst.mkdir(exist_ok=dirs_exist_ok)
892-
for child in children:
893-
if not (ignore and ignore(child)):
894-
stack.append((child, dst.joinpath(child.name)))
895-
if preserve_metadata:
896-
src._copy_metadata(dst)
897-
else:
898-
src._copy_file(dst)
899-
if preserve_metadata:
900-
src._copy_metadata(dst)
901-
except OSError as err:
902-
if on_error is None:
903-
raise
904-
on_error(err)
878+
if not follow_symlinks and src.is_symlink():
879+
dst._symlink_to_target_of(src)
880+
if preserve_metadata:
881+
src._copy_metadata(dst, follow_symlinks=False)
882+
elif src.is_dir():
883+
children = src.iterdir()
884+
dst.mkdir(exist_ok=dirs_exist_ok)
885+
stack.extend((child, dst.joinpath(child.name))
886+
for child in children)
887+
if preserve_metadata:
888+
src._copy_metadata(dst)
889+
else:
890+
src._copy_file(dst)
891+
if preserve_metadata:
892+
src._copy_metadata(dst)
905893
return target
906894

907895
def copy_into(self, target_dir, *, follow_symlinks=True,
908-
dirs_exist_ok=False, preserve_metadata=False, ignore=None,
909-
on_error=None):
896+
dirs_exist_ok=False, preserve_metadata=False):
910897
"""
911898
Copy this file or directory tree into the given existing directory.
912899
"""
@@ -919,8 +906,7 @@ def copy_into(self, target_dir, *, follow_symlinks=True,
919906
target = self.with_segments(target_dir, name)
920907
return self.copy(target, follow_symlinks=follow_symlinks,
921908
dirs_exist_ok=dirs_exist_ok,
922-
preserve_metadata=preserve_metadata, ignore=ignore,
923-
on_error=on_error)
909+
preserve_metadata=preserve_metadata)
924910

925911
def rename(self, target):
926912
"""

Lib/test/test_pathlib/test_pathlib.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -776,11 +776,6 @@ def test_copy_dir_no_read_permission(self):
776776
target = base / 'copyE'
777777
self.assertRaises(PermissionError, source.copy, target)
778778
self.assertFalse(target.exists())
779-
errors = []
780-
source.copy(target, on_error=errors.append)
781-
self.assertEqual(len(errors), 1)
782-
self.assertIsInstance(errors[0], PermissionError)
783-
self.assertFalse(target.exists())
784779

785780
def test_copy_dir_preserve_metadata(self):
786781
base = self.cls(self.base)

Lib/test/test_pathlib/test_pathlib_abc.py

Lines changed: 0 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,14 +1984,6 @@ def test_copy_dir_to_itself(self):
19841984
self.assertRaises(OSError, source.copy, source)
19851985
self.assertRaises(OSError, source.copy, source, follow_symlinks=False)
19861986

1987-
def test_copy_dir_to_itself_on_error(self):
1988-
base = self.cls(self.base)
1989-
source = base / 'dirC'
1990-
errors = []
1991-
source.copy(source, on_error=errors.append)
1992-
self.assertEqual(len(errors), 1)
1993-
self.assertIsInstance(errors[0], OSError)
1994-
19951987
def test_copy_dir_into_itself(self):
19961988
base = self.cls(self.base)
19971989
source = base / 'dirC'
@@ -2000,61 +1992,6 @@ def test_copy_dir_into_itself(self):
20001992
self.assertRaises(OSError, source.copy, target, follow_symlinks=False)
20011993
self.assertFalse(target.exists())
20021994

2003-
def test_copy_missing_on_error(self):
2004-
base = self.cls(self.base)
2005-
source = base / 'foo'
2006-
target = base / 'copyA'
2007-
errors = []
2008-
result = source.copy(target, on_error=errors.append)
2009-
self.assertEqual(result, target)
2010-
self.assertEqual(len(errors), 1)
2011-
self.assertIsInstance(errors[0], FileNotFoundError)
2012-
2013-
def test_copy_dir_ignore_false(self):
2014-
base = self.cls(self.base)
2015-
source = base / 'dirC'
2016-
target = base / 'copyC'
2017-
ignores = []
2018-
def ignore_false(path):
2019-
ignores.append(path)
2020-
return False
2021-
result = source.copy(target, ignore=ignore_false)
2022-
self.assertEqual(result, target)
2023-
self.assertEqual(set(ignores), {
2024-
source / 'dirD',
2025-
source / 'dirD' / 'fileD',
2026-
source / 'fileC',
2027-
source / 'novel.txt',
2028-
})
2029-
self.assertTrue(target.is_dir())
2030-
self.assertTrue(target.joinpath('dirD').is_dir())
2031-
self.assertTrue(target.joinpath('dirD', 'fileD').is_file())
2032-
self.assertEqual(target.joinpath('dirD', 'fileD').read_text(),
2033-
"this is file D\n")
2034-
self.assertTrue(target.joinpath('fileC').is_file())
2035-
self.assertTrue(target.joinpath('fileC').read_text(),
2036-
"this is file C\n")
2037-
2038-
def test_copy_dir_ignore_true(self):
2039-
base = self.cls(self.base)
2040-
source = base / 'dirC'
2041-
target = base / 'copyC'
2042-
ignores = []
2043-
def ignore_true(path):
2044-
ignores.append(path)
2045-
return True
2046-
result = source.copy(target, ignore=ignore_true)
2047-
self.assertEqual(result, target)
2048-
self.assertEqual(set(ignores), {
2049-
source / 'dirD',
2050-
source / 'fileC',
2051-
source / 'novel.txt',
2052-
})
2053-
self.assertTrue(target.is_dir())
2054-
self.assertFalse(target.joinpath('dirD').exists())
2055-
self.assertFalse(target.joinpath('fileC').exists())
2056-
self.assertFalse(target.joinpath('novel.txt').exists())
2057-
20581995
@needs_symlinks
20591996
def test_copy_dangling_symlink(self):
20601997
base = self.cls(self.base)

0 commit comments

Comments
 (0)