Skip to content

Commit

Permalink
pythonGH-128520: More consistent type-checking behaviour in pathlib
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
barneygale committed Feb 16, 2025
1 parent 655fc8a commit 81a26f7
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 21 deletions.
15 changes: 8 additions & 7 deletions Lib/pathlib/_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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:
Expand All @@ -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):
Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand Down
29 changes: 15 additions & 14 deletions Lib/pathlib/_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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):
"""
Expand All @@ -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)
Expand All @@ -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.
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand Down

0 comments on commit 81a26f7

Please sign in to comment.