Disagreement between `PureWindowsPath.relative_to()` and `ntpath.relpath()` with rootless drives #99029

barneygale · 2022-11-02T23:21:35Z

On Windows, the path C: means "the current directory on the C: drive".

But pathlib's relative_to() treats it as the immediate parent of C:\. This makes sense lexographically, but it's inconsistent with everything else:

C:\Users\Barney>python
>>> import ntpath, pathlib
>>> ntpath.relpath('C:/Windows', 'C:/Users/Barney')
'..\\..\\Windows'
>>> ntpath.relpath('C:/Windows', '.')
'..\\..\\Windows'
>>> ntpath.relpath('C:/Windows', 'C:')
'..\\..\\Windows'
>>> pathlib.Path('C:/Windows').relative_to('C:/Users/Barney', walk_up=True)
WindowsPath('../../Windows')
>>> pathlib.Path('C:/Windows').relative_to('.', walk_up=True)
ValueError: One path is relative and the other is absolute.
>>> pathlib.Path('C:/Windows').relative_to('C:')
WindowsPath('/Windows')  # should be ValueError, as we're mixing absolute + relative paths

This prevents us from using relpath() from pathlib, and renders the two implementations incompatible. Booo! Also prevents us from simplifying is_relative_to() down to other == self or other in self.parents

Special cases aren't special enough to break the rules. Let's make these compatible!

Previous discussion: #84538

PR: gh-99029: Fix handling of PureWindowsPath('C:\<blah>').relative_to('C:') #99031

The text was updated successfully, but these errors were encountered:

…e_to('C:')` `relative_to()` now treats naked drive paths as relative. This brings its behaviour in line with other parts of pathlib, and with `ntpath.relpath()`, and so allows us to factor out the pathlib-specific implementation.

brettcannon · 2022-11-04T22:05:18Z

@zooba @eryksun any reason to not standardize on os.path.relpath() instead of pathlib's custom code?

zooba · 2022-11-07T20:33:11Z

Pathlib uses a pure implementation that doesn't rely on global state, such as the current working directory or current operating system (the implementation is in PurePath.relative_to). We don't resolve symlinks or junctions here, so why would we combine it with CWD?

If anything, we should make relative_to raise because C: is a relative path, and it requires an absolute path. There's a note saying that it treats a pathless root like this on purpose, but it's in a comment and not the docstring.

Provided walk_up=True doesn't affect this result, the result is correct. Anywhere on the C drive combined with \Windows will get to the right path - if it gets ..\..\Windows then it's incorrect. But I didn't try this because it's new in 3.12 and so we can change the behaviour if we feel like it.

barneygale · 2022-11-07T20:43:23Z

If anything, we should make relative_to raise because C: is a relative path, and it requires an absolute path. There's a note saying that it treats a pathless root like this on purpose, but it's in a comment and not the docstring.

Indeed. This seems to be the only case where a.relative_path(b) allows a and b to have different anchors (e.g. one relative, one absolute; or different Windows drives); in all other cases, ValueError is raised. My suggestion here is that we make Path('C:/Windows').relative_to('C:') also raise.

I've logged #99199 to cover relpath() calling abspath(). It doesn't cause us any problems for us in pathlib, though (see discussion on #99031). It's "wrong" but has no observable effect, as far as I can tell.

barneygale · 2022-11-12T06:15:51Z

I found another incompatibility between PurePath.relative_to() and os.path.relpath() - see #99199 (comment)

This bug is still valid, but we can't fix it by using relpath(). I've revised my PR.

…C:')` (GH-99031) `relative_to()` now treats naked drive paths as relative. This brings its behaviour in line with other parts of pathlib, and with `ntpath.relpath()`, and so allows us to factor out the pathlib-specific implementation.

barneygale added the type-bug An unexpected behavior, bug, or error label Nov 2, 2022

bedevere-bot mentioned this issue Nov 2, 2022

gh-99029: Fix handling of PureWindowsPath('C:\<blah>').relative_to('C:') #99031

Merged

barneygale mentioned this issue Nov 3, 2022

gh-84538: add strict argument to pathlib.PurePath.relative_to #19813

Merged

mdboom added OS-windows expert-pathlib labels Nov 3, 2022

brettcannon closed this as completed Nov 25, 2022

Oct	NOV	Dec
	27
2021	2022	2023

Disagreement between `PureWindowsPath.relative_to()` and `ntpath.relpath()` with rootless drives #99029

Disagreement between `PureWindowsPath.relative_to()` and `ntpath.relpath()` with rootless drives #99029

barneygale commented Nov 2, 2022 •

edited

brettcannon commented Nov 4, 2022

zooba commented Nov 7, 2022

barneygale commented Nov 7, 2022 •

edited

barneygale commented Nov 12, 2022

Disagreement between PureWindowsPath.relative_to() and ntpath.relpath() with rootless drives #99029

Disagreement between PureWindowsPath.relative_to() and ntpath.relpath() with rootless drives #99029

Comments

barneygale commented Nov 2, 2022 • edited

brettcannon commented Nov 4, 2022

zooba commented Nov 7, 2022

barneygale commented Nov 7, 2022 • edited

barneygale commented Nov 12, 2022

Disagreement between `PureWindowsPath.relative_to()` and `ntpath.relpath()` with rootless drives #99029

Disagreement between `PureWindowsPath.relative_to()` and `ntpath.relpath()` with rootless drives #99029

barneygale commented Nov 2, 2022 •

edited

barneygale commented Nov 7, 2022 •

edited