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.

diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py
index e6ff3fe1187512e9a9c4d030aec2fda59cc59c4e..7de2bb066f8f9929b33049545703906355b2b700 100644 (file)
--- a/Lib/pathlib/_abc.py
+++ b/Lib/pathlib/_abc.py
@@ -573,30 +573,3 @@ class PathBase(PurePathBase):
         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 c5721a69d0047018b234d1accd43299109d1cb92..1da85ddea24376ffba37c5745844bc09200efb7a 100644 (file)
--- a/Lib/pathlib/_local.py
+++ b/Lib/pathlib/_local.py
@@ -1128,16 +1128,38 @@ class Path(PathBase, PurePath):
         """
         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 d13daf8ac8cb07636feab76f3f41dd45b7f859af..a67a1c531630a13a9774af2e778161c6d83ed76c 100644 (file)
--- a/Lib/test/test_pathlib/test_pathlib.py
+++ b/Lib/test/test_pathlib/test_pathlib.py
@@ -1423,26 +1423,97 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest):
         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 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest):
     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 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest):
         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 d588442bd11785a48596cdd6f0c8c29b23ce74e7..0762f224fc9ac4bf4c6ed3d0669983d48a89fbac 100644 (file)
--- a/Lib/test/test_pathlib/test_pathlib_abc.py
+++ b/Lib/test/test_pathlib/test_pathlib_abc.py
@@ -1345,93 +1345,6 @@ class DummyPathTest(DummyPurePathTest):
         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 @@ class DummyPathTest(DummyPurePathTest):
             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