From 81a26f72476f24cfc2bb061ecd55ab75d09b1028 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sun, 16 Feb 2025 17:39:12 +0000 Subject: [PATCH 1/5] GH-128520: More consistent type-checking behaviour in pathlib In the following methods, skip casting of the argument to a path object if the argument has a `with_segments` attribute. In `PurePath`: `relative_to()`, `is_relative_to()`, `match()`, and `full_match()`. In `Path`: `rename()`, `replace()`, `copy()`, `copy_into()`, `move()`, and `move_into()`. Previously the check varied a bit from method to method. The `PurePath` methods used `isinstance(arg, PurePath)`; the `rename()` and `replace()` methods always cast, and the remaining `Path` methods checked for a private `_copy_writer` attribute. We apply identical changes to relevant methods of the private ABCs. This improves performance a bit, because `isinstance()` checks on ABCs are expensive. --- Lib/pathlib/_abc.py | 15 ++++++++------- Lib/pathlib/_local.py | 29 +++++++++++++++-------------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index 501f852059010f..503d4a13b9d69c 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -192,7 +192,7 @@ def full_match(self, pattern, *, case_sensitive=None): Return True if this path matches the given glob-style pattern. The pattern is matched against the entire path. """ - if not isinstance(pattern, JoinablePath): + if not hasattr(pattern, 'with_segments'): pattern = self.with_segments(pattern) if case_sensitive is None: case_sensitive = self.parser.normcase('Aa') == 'Aa' @@ -286,7 +286,7 @@ def glob(self, pattern, *, case_sensitive=None, recurse_symlinks=True): """Iterate over this subtree and yield all existing files (of any kind, including directories) matching the given relative pattern. """ - if not isinstance(pattern, JoinablePath): + if not hasattr(pattern, 'with_segments'): pattern = self.with_segments(pattern) anchor, parts = _explode_path(pattern) if anchor: @@ -307,9 +307,10 @@ def rglob(self, pattern, *, case_sensitive=None, recurse_symlinks=True): directories) matching the given relative pattern, anywhere in this subtree. """ - if not isinstance(pattern, JoinablePath): - pattern = self.with_segments(pattern) - pattern = '**' / pattern + if hasattr(pattern, 'with_segments'): + pattern = '**' / pattern + else: + pattern = self.with_segments('**', pattern) return self.glob(pattern, case_sensitive=case_sensitive, recurse_symlinks=recurse_symlinks) def walk(self, top_down=True, on_error=None, follow_symlinks=False): @@ -360,7 +361,7 @@ def copy(self, target, follow_symlinks=True, dirs_exist_ok=False, """ Recursively copy this file or directory tree to the given destination. """ - if not hasattr(target, '_copy_writer'): + if not hasattr(target, 'with_segments'): target = self.with_segments(target) # Delegate to the target path's CopyWriter object. @@ -378,7 +379,7 @@ def copy_into(self, target_dir, *, follow_symlinks=True, name = self.name if not name: raise ValueError(f"{self!r} has an empty name") - elif hasattr(target_dir, '_copy_writer'): + if hasattr(target_dir, 'with_segments'): target = target_dir / name else: target = self.with_segments(target_dir, name) diff --git a/Lib/pathlib/_local.py b/Lib/pathlib/_local.py index 2f0c87fd27b624..cdb3036c8d5322 100644 --- a/Lib/pathlib/_local.py +++ b/Lib/pathlib/_local.py @@ -475,7 +475,7 @@ def relative_to(self, other, *, walk_up=False): The *walk_up* parameter controls whether `..` may be used to resolve the path. """ - if not isinstance(other, PurePath): + if not hasattr(other, 'with_segments'): other = self.with_segments(other) for step, path in enumerate(chain([other], other.parents)): if path == self or path in self.parents: @@ -492,7 +492,7 @@ def relative_to(self, other, *, walk_up=False): def is_relative_to(self, other): """Return True if the path is relative to another path or False. """ - if not isinstance(other, PurePath): + if not hasattr(other, 'with_segments'): other = self.with_segments(other) return other == self or other in self.parents @@ -545,7 +545,7 @@ def full_match(self, pattern, *, case_sensitive=None): Return True if this path matches the given glob-style pattern. The pattern is matched against the entire path. """ - if not isinstance(pattern, PurePath): + if not hasattr(pattern, 'with_segments'): pattern = self.with_segments(pattern) if case_sensitive is None: case_sensitive = self.parser is posixpath @@ -564,7 +564,7 @@ def match(self, path_pattern, *, case_sensitive=None): is matched. The recursive wildcard '**' is *not* supported by this method. """ - if not isinstance(path_pattern, PurePath): + if not hasattr(path_pattern, 'with_segments'): path_pattern = self.with_segments(path_pattern) if case_sensitive is None: case_sensitive = self.parser is posixpath @@ -1064,7 +1064,9 @@ def rename(self, target): Returns the new Path instance pointing to the target path. """ os.rename(self, target) - return self.with_segments(target) + if not hasattr(target, 'with_segments'): + target = self.with_segments(target) + return target def replace(self, target): """ @@ -1077,7 +1079,9 @@ def replace(self, target): Returns the new Path instance pointing to the target path. """ os.replace(self, target) - return self.with_segments(target) + if not hasattr(target, 'with_segments'): + target = self.with_segments(target) + return target _copy_reader = property(LocalCopyReader) _copy_writer = property(LocalCopyWriter) @@ -1087,7 +1091,7 @@ def copy(self, target, follow_symlinks=True, dirs_exist_ok=False, """ Recursively copy this file or directory tree to the given destination. """ - if not hasattr(target, '_copy_writer'): + if not hasattr(target, 'with_segments'): target = self.with_segments(target) # Delegate to the target path's CopyWriter object. @@ -1105,7 +1109,7 @@ def copy_into(self, target_dir, *, follow_symlinks=True, name = self.name if not name: raise ValueError(f"{self!r} has an empty name") - elif hasattr(target_dir, '_copy_writer'): + elif hasattr(target_dir, 'with_segments'): target = target_dir / name else: target = self.with_segments(target_dir, name) @@ -1119,16 +1123,13 @@ def move(self, target): """ # Use os.replace() if the target is os.PathLike and on the same FS. try: - target_str = os.fspath(target) + target = self.with_segments(target) except TypeError: pass else: - if not hasattr(target, '_copy_writer'): - target = self.with_segments(target_str) target._copy_writer._ensure_different_file(self) try: - os.replace(self, target_str) - return target + return self.replace(target) except OSError as err: if err.errno != EXDEV: raise @@ -1144,7 +1145,7 @@ def move_into(self, target_dir): name = self.name if not name: raise ValueError(f"{self!r} has an empty name") - elif hasattr(target_dir, '_copy_writer'): + elif hasattr(target_dir, 'with_segments'): target = target_dir / name else: target = self.with_segments(target_dir, name) From 7c82d27403b9e2423890ffac7d8baa8f76b1f5b4 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sun, 16 Feb 2025 18:04:10 +0000 Subject: [PATCH 2/5] if --> elif --- Lib/pathlib/_abc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index 503d4a13b9d69c..9d2fd1a0dde718 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -379,7 +379,7 @@ def copy_into(self, target_dir, *, follow_symlinks=True, name = self.name if not name: raise ValueError(f"{self!r} has an empty name") - if hasattr(target_dir, 'with_segments'): + elif hasattr(target_dir, 'with_segments'): target = target_dir / name else: target = self.with_segments(target_dir, name) From baedf32d2adf10f382f48e596411dde7beca32fb Mon Sep 17 00:00:00 2001 From: barneygale Date: Sun, 16 Feb 2025 18:13:45 +0000 Subject: [PATCH 3/5] Add news blurb --- .../Library/2025-02-16-18-13-40.gh-issue-128520.iZtOMz.rst | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-02-16-18-13-40.gh-issue-128520.iZtOMz.rst diff --git a/Misc/NEWS.d/next/Library/2025-02-16-18-13-40.gh-issue-128520.iZtOMz.rst b/Misc/NEWS.d/next/Library/2025-02-16-18-13-40.gh-issue-128520.iZtOMz.rst new file mode 100644 index 00000000000000..15de99e8bbdbcd --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-02-16-18-13-40.gh-issue-128520.iZtOMz.rst @@ -0,0 +1,5 @@ +Apply type conversion consistently in :class:`pathlib.PurePath` and +:class:`~pathlib.Path` methods can accept a path object as an argument, such +as :meth:`~pathlib.PurePath.match` and :meth:`~pathlib.Path.rename`. The +argument is now converted to path object if it lacks a +:meth:`~pathlib.PurePath.with_segments` attribute, and not otherwise. From 89e05e4aff55fef05c96f7f1eabd6e144ef50ee8 Mon Sep 17 00:00:00 2001 From: barneygale Date: Mon, 17 Feb 2025 19:24:52 +0000 Subject: [PATCH 4/5] Simplify _abc checking --- Lib/pathlib/_abc.py | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index 115e120572d9f1..f7407468bff47a 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -17,13 +17,13 @@ from pathlib._os import magic_open, CopyWriter -def _explode_path(path): +def _explode_path(path, parser): """ Split the path into a 2-tuple (anchor, parts), where *anchor* is the uppermost parent of the path (equivalent to path.parents[-1]), and *parts* is a reversed list of parts following the anchor. """ - split = path.parser.split + split = parser.split path = str(path) parent, name = split(path) names = [] @@ -68,7 +68,7 @@ def __str__(self): @property def anchor(self): """The concatenation of the drive and root, or ''.""" - return _explode_path(self)[0] + return _explode_path(self, self.parser)[0] @property def name(self): @@ -140,7 +140,7 @@ def with_suffix(self, suffix): def parts(self): """An object providing sequence-like access to the components in the filesystem path.""" - anchor, parts = _explode_path(self) + anchor, parts = _explode_path(self, self.parser) if anchor: parts.append(anchor) return tuple(reversed(parts)) @@ -192,12 +192,12 @@ def full_match(self, pattern, *, case_sensitive=None): Return True if this path matches the given glob-style pattern. The pattern is matched against the entire path. """ - if not hasattr(pattern, 'with_segments'): - pattern = self.with_segments(pattern) if case_sensitive is None: case_sensitive = self.parser.normcase('Aa') == 'Aa' - globber = _PathGlobber(pattern.parser.sep, case_sensitive, recursive=True) - match = globber.compile(str(pattern)) + sep = self.parser.sep + anchor, parts = _explode_path(pattern, self.parser) + globber = _PathGlobber(sep, case_sensitive, recursive=True) + match = globber.compile(anchor + sep.join(reversed(parts))) return match(str(self)) is not None @@ -286,9 +286,7 @@ def glob(self, pattern, *, case_sensitive=None, recurse_symlinks=True): """Iterate over this subtree and yield all existing files (of any kind, including directories) matching the given relative pattern. """ - if not hasattr(pattern, 'with_segments'): - pattern = self.with_segments(pattern) - anchor, parts = _explode_path(pattern) + anchor, parts = _explode_path(pattern, self.parser) if anchor: raise NotImplementedError("Non-relative patterns are unsupported") case_sensitive_default = self.parser.normcase('Aa') == 'Aa' @@ -348,9 +346,6 @@ def copy(self, target, follow_symlinks=True, dirs_exist_ok=False, """ Recursively copy this file or directory tree to the given destination. """ - if not hasattr(target, 'with_segments'): - target = self.with_segments(target) - # Delegate to the target path's CopyWriter object. try: create = target._copy_writer._create @@ -366,10 +361,8 @@ def copy_into(self, target_dir, *, follow_symlinks=True, name = self.name if not name: raise ValueError(f"{self!r} has an empty name") - elif hasattr(target_dir, 'with_segments'): - target = target_dir / name else: - target = self.with_segments(target_dir, name) + target = target_dir / name return self.copy(target, follow_symlinks=follow_symlinks, dirs_exist_ok=dirs_exist_ok, preserve_metadata=preserve_metadata) From 2616d5442893141320cb29649dda0615c7bca98e Mon Sep 17 00:00:00 2001 From: barneygale Date: Mon, 17 Feb 2025 19:41:45 +0000 Subject: [PATCH 5/5] Revert "Simplify _abc checking" This reverts commit 89e05e4aff55fef05c96f7f1eabd6e144ef50ee8. --- Lib/pathlib/_abc.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index f7407468bff47a..115e120572d9f1 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -17,13 +17,13 @@ from pathlib._os import magic_open, CopyWriter -def _explode_path(path, parser): +def _explode_path(path): """ Split the path into a 2-tuple (anchor, parts), where *anchor* is the uppermost parent of the path (equivalent to path.parents[-1]), and *parts* is a reversed list of parts following the anchor. """ - split = parser.split + split = path.parser.split path = str(path) parent, name = split(path) names = [] @@ -68,7 +68,7 @@ def __str__(self): @property def anchor(self): """The concatenation of the drive and root, or ''.""" - return _explode_path(self, self.parser)[0] + return _explode_path(self)[0] @property def name(self): @@ -140,7 +140,7 @@ def with_suffix(self, suffix): def parts(self): """An object providing sequence-like access to the components in the filesystem path.""" - anchor, parts = _explode_path(self, self.parser) + anchor, parts = _explode_path(self) if anchor: parts.append(anchor) return tuple(reversed(parts)) @@ -192,12 +192,12 @@ def full_match(self, pattern, *, case_sensitive=None): Return True if this path matches the given glob-style pattern. The pattern is matched against the entire path. """ + if not hasattr(pattern, 'with_segments'): + pattern = self.with_segments(pattern) if case_sensitive is None: case_sensitive = self.parser.normcase('Aa') == 'Aa' - sep = self.parser.sep - anchor, parts = _explode_path(pattern, self.parser) - globber = _PathGlobber(sep, case_sensitive, recursive=True) - match = globber.compile(anchor + sep.join(reversed(parts))) + globber = _PathGlobber(pattern.parser.sep, case_sensitive, recursive=True) + match = globber.compile(str(pattern)) return match(str(self)) is not None @@ -286,7 +286,9 @@ def glob(self, pattern, *, case_sensitive=None, recurse_symlinks=True): """Iterate over this subtree and yield all existing files (of any kind, including directories) matching the given relative pattern. """ - anchor, parts = _explode_path(pattern, self.parser) + if not hasattr(pattern, 'with_segments'): + pattern = self.with_segments(pattern) + anchor, parts = _explode_path(pattern) if anchor: raise NotImplementedError("Non-relative patterns are unsupported") case_sensitive_default = self.parser.normcase('Aa') == 'Aa' @@ -346,6 +348,9 @@ def copy(self, target, follow_symlinks=True, dirs_exist_ok=False, """ Recursively copy this file or directory tree to the given destination. """ + if not hasattr(target, 'with_segments'): + target = self.with_segments(target) + # Delegate to the target path's CopyWriter object. try: create = target._copy_writer._create @@ -361,8 +366,10 @@ def copy_into(self, target_dir, *, follow_symlinks=True, name = self.name if not name: raise ValueError(f"{self!r} has an empty name") - else: + elif hasattr(target_dir, 'with_segments'): target = target_dir / name + else: + 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)