Skip to content

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

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

Merged
merged 1 commit into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions Doc/library/pathlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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*.
Expand All @@ -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
Expand Down
52 changes: 19 additions & 33 deletions Lib/pathlib/_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -865,48 +865,35 @@ def _copy_file(self, target):
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.
"""
Expand All @@ -919,8 +906,7 @@ def copy_into(self, target_dir, *, follow_symlinks=True,
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):
"""
Expand Down
5 changes: 0 additions & 5 deletions Lib/test/test_pathlib/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -776,11 +776,6 @@ def test_copy_dir_no_read_permission(self):
target = base / 'copyE'
self.assertRaises(PermissionError, source.copy, target)
self.assertFalse(target.exists())
errors = []
source.copy(target, on_error=errors.append)
self.assertEqual(len(errors), 1)
self.assertIsInstance(errors[0], PermissionError)
self.assertFalse(target.exists())

def test_copy_dir_preserve_metadata(self):
base = self.cls(self.base)
Expand Down
63 changes: 0 additions & 63 deletions Lib/test/test_pathlib/test_pathlib_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1984,14 +1984,6 @@ def test_copy_dir_to_itself(self):
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'
Expand All @@ -2000,61 +1992,6 @@ def test_copy_dir_into_itself(self):
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)
Expand Down
Loading