Skip to content
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

GH-127381: pathlib ABCs: remove PathBase.move() and move_into() #128337

Merged
merged 2 commits into from
Jan 4, 2025
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
27 changes: 0 additions & 27 deletions Lib/pathlib/_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -573,30 +573,3 @@ def copy_into(self, target_dir, *, follow_symlinks=True,
return self.copy(target, follow_symlinks=follow_symlinks,
dirs_exist_ok=dirs_exist_ok,
preserve_metadata=preserve_metadata)

def _delete(self):
"""
Delete this file or directory (including all sub-directories).
"""
raise NotImplementedError

def move(self, target):
"""
Recursively move this file or directory tree to the given destination.
"""
target = self.copy(target, follow_symlinks=False, preserve_metadata=True)
self._delete()
return target

def move_into(self, target_dir):
"""
Move this file or directory tree into the given existing directory.
"""
name = self.name
if not name:
raise ValueError(f"{self!r} has an empty name")
elif isinstance(target_dir, PathBase):
target = target_dir / name
else:
target = self.with_segments(target_dir, name)
return self.move(target)
38 changes: 30 additions & 8 deletions Lib/pathlib/_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -1128,16 +1128,38 @@ def move(self, target):
"""
Recursively move this file or directory tree to the given destination.
"""
if not isinstance(target, PathBase):
target = self.with_segments(target)
target.copy._ensure_different_file(self)
# Use os.replace() if the target is os.PathLike and on the same FS.
try:
return self.replace(target)
except OSError as err:
if err.errno != EXDEV:
raise
target_str = os.fspath(target)
except TypeError:
pass
else:
if not isinstance(target, PathBase):
target = self.with_segments(target_str)
target.copy._ensure_different_file(self)
try:
os.replace(self, target_str)
return target
except OSError as err:
if err.errno != EXDEV:
raise
# Fall back to copy+delete.
return PathBase.move(self, target)
target = self.copy(target, follow_symlinks=False, preserve_metadata=True)
self._delete()
return target

def move_into(self, target_dir):
"""
Move this file or directory tree into the given existing directory.
"""
name = self.name
if not name:
raise ValueError(f"{self!r} has an empty name")
elif isinstance(target_dir, PathBase):
target = target_dir / name
else:
target = self.with_segments(target_dir, name)
return self.move(target)

if hasattr(os, "symlink"):
def symlink_to(self, target, target_is_directory=False):
Expand Down
118 changes: 118 additions & 0 deletions Lib/test/test_pathlib/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1423,26 +1423,97 @@ def test_move_dangling_symlink(self):
self.assertTrue(target.is_symlink())
self.assertEqual(source_readlink, target.readlink())

def test_move_file(self):
base = self.cls(self.base)
source = base / 'fileA'
source_text = source.read_text()
target = base / 'fileA_moved'
result = source.move(target)
self.assertEqual(result, target)
self.assertFalse(source.exists())
self.assertTrue(target.exists())
self.assertEqual(source_text, target.read_text())

@patch_replace
def test_move_file_other_fs(self):
self.test_move_file()

def test_move_file_to_file(self):
base = self.cls(self.base)
source = base / 'fileA'
source_text = source.read_text()
target = base / 'dirB' / 'fileB'
result = source.move(target)
self.assertEqual(result, target)
self.assertFalse(source.exists())
self.assertTrue(target.exists())
self.assertEqual(source_text, target.read_text())

@patch_replace
def test_move_file_to_file_other_fs(self):
self.test_move_file_to_file()

def test_move_file_to_dir(self):
base = self.cls(self.base)
source = base / 'fileA'
target = base / 'dirB'
self.assertRaises(OSError, source.move, target)

@patch_replace
def test_move_file_to_dir_other_fs(self):
self.test_move_file_to_dir()

def test_move_file_to_itself(self):
base = self.cls(self.base)
source = base / 'fileA'
self.assertRaises(OSError, source.move, source)

def test_move_dir(self):
base = self.cls(self.base)
source = base / 'dirC'
target = base / 'dirC_moved'
result = source.move(target)
self.assertEqual(result, target)
self.assertFalse(source.exists())
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")

@patch_replace
def test_move_dir_other_fs(self):
self.test_move_dir()

def test_move_dir_to_dir(self):
base = self.cls(self.base)
source = base / 'dirC'
target = base / 'dirB'
self.assertRaises(OSError, source.move, target)
self.assertTrue(source.exists())
self.assertTrue(target.exists())

@patch_replace
def test_move_dir_to_dir_other_fs(self):
self.test_move_dir_to_dir()

def test_move_dir_to_itself(self):
base = self.cls(self.base)
source = base / 'dirC'
self.assertRaises(OSError, source.move, source)
self.assertTrue(source.exists())

def test_move_dir_into_itself(self):
base = self.cls(self.base)
source = base / 'dirC'
target = base / 'dirC' / 'bar'
self.assertRaises(OSError, source.move, target)
self.assertTrue(source.exists())
self.assertFalse(target.exists())

@patch_replace
def test_move_dir_into_itself_other_fs(self):
self.test_move_dir_into_itself()
Expand Down Expand Up @@ -1472,10 +1543,26 @@ def test_move_dir_symlink_to_itself_other_fs(self):
def test_move_dangling_symlink_other_fs(self):
self.test_move_dangling_symlink()

def test_move_into(self):
base = self.cls(self.base)
source = base / 'fileA'
source_text = source.read_text()
target_dir = base / 'dirA'
result = source.move_into(target_dir)
self.assertEqual(result, target_dir / 'fileA')
self.assertFalse(source.exists())
self.assertTrue(result.exists())
self.assertEqual(source_text, result.read_text())

@patch_replace
def test_move_into_other_os(self):
self.test_move_into()

def test_move_into_empty_name(self):
source = self.cls('')
target_dir = self.base
self.assertRaises(ValueError, source.move_into, target_dir)

@patch_replace
def test_move_into_empty_name_other_os(self):
self.test_move_into_empty_name()
Expand Down Expand Up @@ -1794,6 +1881,37 @@ def test_rmdir(self):
self.assertFileNotFound(p.stat)
self.assertFileNotFound(p.unlink)

def test_delete_file(self):
p = self.cls(self.base) / 'fileA'
p._delete()
self.assertFalse(p.exists())
self.assertFileNotFound(p._delete)

def test_delete_dir(self):
base = self.cls(self.base)
base.joinpath('dirA')._delete()
self.assertFalse(base.joinpath('dirA').exists())
self.assertFalse(base.joinpath('dirA', 'linkC').exists(
follow_symlinks=False))
base.joinpath('dirB')._delete()
self.assertFalse(base.joinpath('dirB').exists())
self.assertFalse(base.joinpath('dirB', 'fileB').exists())
self.assertFalse(base.joinpath('dirB', 'linkD').exists(
follow_symlinks=False))
base.joinpath('dirC')._delete()
self.assertFalse(base.joinpath('dirC').exists())
self.assertFalse(base.joinpath('dirC', 'dirD').exists())
self.assertFalse(base.joinpath('dirC', 'dirD', 'fileD').exists())
self.assertFalse(base.joinpath('dirC', 'fileC').exists())
self.assertFalse(base.joinpath('dirC', 'novel.txt').exists())

def test_delete_missing(self):
tmp = self.cls(self.base, 'delete')
tmp.mkdir()
# filename is guaranteed not to exist
filename = tmp / 'foo'
self.assertRaises(FileNotFoundError, filename._delete)

@needs_symlinks
def test_delete_symlink(self):
tmp = self.cls(self.base, 'delete')
Expand Down
118 changes: 0 additions & 118 deletions Lib/test/test_pathlib/test_pathlib_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1345,93 +1345,6 @@ def test_copy_into_empty_name(self):
target_dir = self.base
self.assertRaises(ValueError, source.copy_into, target_dir)

def test_move_file(self):
base = self.cls(self.base)
source = base / 'fileA'
source_text = source.read_text()
target = base / 'fileA_moved'
result = source.move(target)
self.assertEqual(result, target)
self.assertFalse(source.exists())
self.assertTrue(target.exists())
self.assertEqual(source_text, target.read_text())

def test_move_file_to_file(self):
base = self.cls(self.base)
source = base / 'fileA'
source_text = source.read_text()
target = base / 'dirB' / 'fileB'
result = source.move(target)
self.assertEqual(result, target)
self.assertFalse(source.exists())
self.assertTrue(target.exists())
self.assertEqual(source_text, target.read_text())

def test_move_file_to_dir(self):
base = self.cls(self.base)
source = base / 'fileA'
target = base / 'dirB'
self.assertRaises(OSError, source.move, target)

def test_move_file_to_itself(self):
base = self.cls(self.base)
source = base / 'fileA'
self.assertRaises(OSError, source.move, source)

def test_move_dir(self):
base = self.cls(self.base)
source = base / 'dirC'
target = base / 'dirC_moved'
result = source.move(target)
self.assertEqual(result, target)
self.assertFalse(source.exists())
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_move_dir_to_dir(self):
base = self.cls(self.base)
source = base / 'dirC'
target = base / 'dirB'
self.assertRaises(OSError, source.move, target)
self.assertTrue(source.exists())
self.assertTrue(target.exists())

def test_move_dir_to_itself(self):
base = self.cls(self.base)
source = base / 'dirC'
self.assertRaises(OSError, source.move, source)
self.assertTrue(source.exists())

def test_move_dir_into_itself(self):
base = self.cls(self.base)
source = base / 'dirC'
target = base / 'dirC' / 'bar'
self.assertRaises(OSError, source.move, target)
self.assertTrue(source.exists())
self.assertFalse(target.exists())

def test_move_into(self):
base = self.cls(self.base)
source = base / 'fileA'
source_text = source.read_text()
target_dir = base / 'dirA'
result = source.move_into(target_dir)
self.assertEqual(result, target_dir / 'fileA')
self.assertFalse(source.exists())
self.assertTrue(result.exists())
self.assertEqual(source_text, result.read_text())

def test_move_into_empty_name(self):
source = self.cls('')
target_dir = self.base
self.assertRaises(ValueError, source.move_into, target_dir)

def test_iterdir(self):
P = self.cls
p = P(self.base)
Expand Down Expand Up @@ -1660,37 +1573,6 @@ def test_is_symlink(self):
self.assertIs((P / 'linkA\udfff').is_file(), False)
self.assertIs((P / 'linkA\x00').is_file(), False)

def test_delete_file(self):
p = self.cls(self.base) / 'fileA'
p._delete()
self.assertFalse(p.exists())
self.assertFileNotFound(p._delete)

def test_delete_dir(self):
base = self.cls(self.base)
base.joinpath('dirA')._delete()
self.assertFalse(base.joinpath('dirA').exists())
self.assertFalse(base.joinpath('dirA', 'linkC').exists(
follow_symlinks=False))
base.joinpath('dirB')._delete()
self.assertFalse(base.joinpath('dirB').exists())
self.assertFalse(base.joinpath('dirB', 'fileB').exists())
self.assertFalse(base.joinpath('dirB', 'linkD').exists(
follow_symlinks=False))
base.joinpath('dirC')._delete()
self.assertFalse(base.joinpath('dirC').exists())
self.assertFalse(base.joinpath('dirC', 'dirD').exists())
self.assertFalse(base.joinpath('dirC', 'dirD', 'fileD').exists())
self.assertFalse(base.joinpath('dirC', 'fileC').exists())
self.assertFalse(base.joinpath('dirC', 'novel.txt').exists())

def test_delete_missing(self):
tmp = self.cls(self.base, 'delete')
tmp.mkdir()
# filename is guaranteed not to exist
filename = tmp / 'foo'
self.assertRaises(FileNotFoundError, filename._delete)


class DummyPathWalkTest(unittest.TestCase):
cls = DummyPath
Expand Down
Loading