Skip to content

Commit

Permalink
pathlib ABCs: add _raw_path property
Browse files Browse the repository at this point in the history
It's wrong for the `PurePathBase` methods to rely so much on `__str__()`.
Instead, they should treat the raw path(s) as opaque objects and leave the
details to `pathmod`.

This commit adds a `PurePathBase._raw_path` property and uses it through
many of the other ABC methods. These methods are all redefined in
`PurePath` and `Path`, so this has no effect on the public classes.
  • Loading branch information
barneygale committed Jan 12, 2024
1 parent c9b8a22 commit 52c25b4
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 22 deletions.
12 changes: 7 additions & 5 deletions Lib/pathlib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,23 +257,25 @@ def _parse_path(cls, path):
parsed = [sys.intern(str(x)) for x in rel.split(sep) if x and x != '.']
return drv, root, parsed

def _load_parts(self):
@property
def _raw_path(self):
"""The joined but unnormalized path."""
paths = self._raw_paths
if len(paths) == 0:
path = ''
elif len(paths) == 1:
path = paths[0]
else:
path = self.pathmod.join(*paths)
self._drv, self._root, self._tail_cached = self._parse_path(path)
return path

@property
def drive(self):
"""The drive prefix (letter or UNC path), if any."""
try:
return self._drv
except AttributeError:
self._load_parts()
self._drv, self._root, self._tail_cached = self._parse_path(self._raw_path)
return self._drv

@property
Expand All @@ -282,15 +284,15 @@ def root(self):
try:
return self._root
except AttributeError:
self._load_parts()
self._drv, self._root, self._tail_cached = self._parse_path(self._raw_path)
return self._root

@property
def _tail(self):
try:
return self._tail_cached
except AttributeError:
self._load_parts()
self._drv, self._root, self._tail_cached = self._parse_path(self._raw_path)
return self._tail_cached

@property
Expand Down
35 changes: 20 additions & 15 deletions Lib/pathlib/_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,15 @@ def with_segments(self, *pathsegments):
"""
return type(self)(*pathsegments)

@property
def _raw_path(self):
"""The joined but unnormalized path."""
return self.pathmod.join(*self._raw_paths)

def __str__(self):
"""Return the string representation of the path, suitable for
passing to system calls."""
return self.pathmod.join(*self._raw_paths)
return self._raw_path

def as_posix(self):
"""Return the string representation of the path with forward (/)
Expand All @@ -177,23 +182,23 @@ def as_posix(self):
@property
def drive(self):
"""The drive prefix (letter or UNC path), if any."""
return self.pathmod.splitdrive(str(self))[0]
return self.pathmod.splitdrive(self._raw_path)[0]

@property
def root(self):
"""The root of the path, if any."""
return self.pathmod.splitroot(str(self))[1]
return self.pathmod.splitroot(self._raw_path)[1]

@property
def anchor(self):
"""The concatenation of the drive and root, or ''."""
drive, root, _ = self.pathmod.splitroot(str(self))
drive, root, _ = self.pathmod.splitroot(self._raw_path)
return drive + root

@property
def name(self):
"""The final path component, if any."""
return self.pathmod.basename(str(self))
return self.pathmod.basename(self._raw_path)

@property
def suffix(self):
Expand Down Expand Up @@ -237,7 +242,7 @@ def with_name(self, name):
dirname = self.pathmod.dirname
if dirname(name):
raise ValueError(f"Invalid name {name!r}")
return self.with_segments(dirname(str(self)), name)
return self.with_segments(dirname(self._raw_path), name)

def with_stem(self, stem):
"""Return a new path with the stem changed."""
Expand Down Expand Up @@ -268,17 +273,17 @@ def relative_to(self, other, *, walk_up=False):
anchor0, parts0 = self._stack
anchor1, parts1 = other._stack
if anchor0 != anchor1:
raise ValueError(f"{str(self)!r} and {str(other)!r} have different anchors")
raise ValueError(f"{self._raw_path!r} and {other._raw_path!r} have different anchors")
while parts0 and parts1 and parts0[-1] == parts1[-1]:
parts0.pop()
parts1.pop()
for part in parts1:
if not part or part == '.':
pass
elif not walk_up:
raise ValueError(f"{str(self)!r} is not in the subpath of {str(other)!r}")
raise ValueError(f"{self._raw_path!r} is not in the subpath of {other._raw_path!r}")
elif part == '..':
raise ValueError(f"'..' segment in {str(other)!r} cannot be walked")
raise ValueError(f"'..' segment in {other._raw_path!r} cannot be walked")
else:
parts0.append('..')
return self.with_segments('', *reversed(parts0))
Expand Down Expand Up @@ -337,7 +342,7 @@ def _stack(self):
*parts* is a reversed list of parts following the anchor.
"""
split = self.pathmod.split
path = str(self)
path = self._raw_path
parent, name = split(path)
names = []
while path != parent:
Expand All @@ -349,7 +354,7 @@ def _stack(self):
@property
def parent(self):
"""The logical parent of the path."""
path = str(self)
path = self._raw_path
parent = self.pathmod.dirname(path)
if path != parent:
parent = self.with_segments(parent)
Expand All @@ -361,7 +366,7 @@ def parent(self):
def parents(self):
"""A sequence of this path's logical parents."""
dirname = self.pathmod.dirname
path = str(self)
path = self._raw_path
parent = dirname(path)
parents = []
while path != parent:
Expand All @@ -383,7 +388,7 @@ def is_absolute(self):
return True
return False
else:
return self.pathmod.isabs(str(self))
return self.pathmod.isabs(self._raw_path)

def is_reserved(self):
"""Return True if the path contains one of the special names reserved
Expand Down Expand Up @@ -898,7 +903,7 @@ def resolve(self, strict=False):
# encountered during resolution.
link_count += 1
if link_count >= self._max_symlinks:
raise OSError(ELOOP, "Too many symbolic links in path", str(self))
raise OSError(ELOOP, "Too many symbolic links in path", self._raw_path)
target_root, target_parts = path.readlink()._stack
# If the symlink target is absolute (like '/etc/hosts'), set the current
# path to its uppermost parent (like '/').
Expand All @@ -912,7 +917,7 @@ def resolve(self, strict=False):
parts.extend(target_parts)
continue
elif parts and not S_ISDIR(st.st_mode):
raise NotADirectoryError(ENOTDIR, "Not a directory", str(self))
raise NotADirectoryError(ENOTDIR, "Not a directory", self._raw_path)
except OSError:
if strict:
raise
Expand Down
10 changes: 10 additions & 0 deletions Lib/test/test_pathlib/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,16 @@ def test_is_relative_to_several_args(self):
with self.assertWarns(DeprecationWarning):
p.is_relative_to('a', 'b')

def test_relative_to_bytes(self):
P = self.cls
p = P('a/b')
self.assertRaises(TypeError, p.relative_to, b'a')

def test_is_relative_to_bytes(self):
P = self.cls
p = P('a/b')
self.assertRaises(TypeError, p.is_relative_to, b'a')

def test_match_empty(self):
P = self.cls
self.assertRaises(ValueError, P('a').match, '')
Expand Down
12 changes: 10 additions & 2 deletions Lib/test/test_pathlib/test_pathlib_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,11 +473,15 @@ def test_with_suffix_seps(self):
self.assertRaises(ValueError, P('a/b').with_suffix, './.d')
self.assertRaises(ValueError, P('a/b').with_suffix, '.d/.')

def test_relative_to_bytes(self):
P = self.cls
p = P('a/b')
self.assertRaises(ValueError, p.relative_to, b'a')

def test_relative_to_common(self):
P = self.cls
p = P('a/b')
self.assertRaises(TypeError, p.relative_to)
self.assertRaises(TypeError, p.relative_to, b'a')
self.assertEqual(p.relative_to(P('')), P('a/b'))
self.assertEqual(p.relative_to(''), P('a/b'))
self.assertEqual(p.relative_to(P('a')), P('b'))
Expand Down Expand Up @@ -548,11 +552,15 @@ def test_relative_to_common(self):
self.assertRaises(ValueError, p.relative_to, P("a/.."), walk_up=True)
self.assertRaises(ValueError, p.relative_to, P("/a/.."), walk_up=True)

def test_is_relative_to_bytes(self):
P = self.cls
p = P('a/b')
self.assertFalse(p.is_relative_to(b'a'))

def test_is_relative_to_common(self):
P = self.cls
p = P('a/b')
self.assertRaises(TypeError, p.is_relative_to)
self.assertRaises(TypeError, p.is_relative_to, b'a')
self.assertTrue(p.is_relative_to(P('')))
self.assertTrue(p.is_relative_to(''))
self.assertTrue(p.is_relative_to(P('a')))
Expand Down

0 comments on commit 52c25b4

Please sign in to comment.