https://github.com/python/cpython/commit/95352dcb9321423d0606ae0b01524ad61c2... commit: 95352dcb9321423d0606ae0b01524ad61c2c2ec1 branch: main author: Barney Gale <barney.gale@gmail.com> committer: barneygale <barney.gale@gmail.com> date: 2025-01-04T12:53:51Z summary: GH-127381: pathlib ABCs: remove `PathBase.move()` and `move_into()` (#128337) These methods combine `_delete()` and `copy()`, but `_delete()` isn't part of the public interface, and it's unlikely to be added until the pathlib ABCs are made official, or perhaps even later. files: M Lib/pathlib/_abc.py M Lib/pathlib/_local.py M Lib/test/test_pathlib/test_pathlib.py M Lib/test/test_pathlib/test_pathlib_abc.py diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index e6ff3fe1187512..7de2bb066f8f99 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -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) diff --git a/Lib/pathlib/_local.py b/Lib/pathlib/_local.py index c5721a69d00470..1da85ddea24376 100644 --- a/Lib/pathlib/_local.py +++ b/Lib/pathlib/_local.py @@ -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): diff --git a/Lib/test/test_pathlib/test_pathlib.py b/Lib/test/test_pathlib/test_pathlib.py index d13daf8ac8cb07..a67a1c531630a1 100644 --- a/Lib/test/test_pathlib/test_pathlib.py +++ b/Lib/test/test_pathlib/test_pathlib.py @@ -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() @@ -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() @@ -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') diff --git a/Lib/test/test_pathlib/test_pathlib_abc.py b/Lib/test/test_pathlib/test_pathlib_abc.py index d588442bd11785..0762f224fc9ac4 100644 --- a/Lib/test/test_pathlib/test_pathlib_abc.py +++ b/Lib/test/test_pathlib/test_pathlib_abc.py @@ -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) @@ -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