GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-84538: add strict argument to pathlib.PurePath.relative_to by domragusa · Pull Request #19813 · python/cpython

Conversation

@domragusa

@domragusa domragusa commented Apr 30, 2020 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

By default, :meth:pathlib.PurePath.relative_to doesn't deal with paths that are not a direct prefix of the other, raising an exception in that instance. This change adds a walk_up parameter that can be set to allow for using .. to calculate the relative path.

example:

>>> p = PurePosixPath('/etc/passwd')
>>> p.relative_to('/etc')
PurePosixPath('passwd')
>>> p.relative_to('/usr')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "pathlib.py", line 940, in relative_to
    raise ValueError(error_message.format(str(self), str(formatted)))
ValueError: '/etc/passwd' does not start with '/usr'
>>> p.relative_to('/usr', strict=False)
PurePosixPath('../etc/passwd')

https://bugs.python.org/issue40358

Automerge-Triggered-By: GH:brettcannon

  • Issue: pathlib's relative_to should behave like os.path.relpath #84538

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

barneygale and Socob reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@domragusa
bpo-40358: add strict param to pathlib.PurePath.relative_to
ec64368
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Apr 30, 2020
@bedevere-bot bedevere-bot added the awaiting review label Apr 30, 2020
@domragusa domragusa mentioned this pull request Apr 30, 2020
bpo-40358: add strict parameter to pathlib.PurePath.relative_to #19807
Closed
CuriousLearner
CuriousLearner reviewed Apr 30, 2020
View reviewed changes
Comment thread Lib/pathlib.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@domragusa
resolve conflicts with pythonGH-19611
6d721a0
@domragusa domragusa changed the title bpo-40358: add strict param to pathlib.PurePath.relative_to bpo-40358: add strict argument to pathlib.PurePath.relative_to May 28, 2020
@zeehio zeehio mentioned this pull request Oct 30, 2020
Add fs.relative_to() function. Closes #2184 mesonbuild/meson#7908
Closed
@zeehio

zeehio commented Oct 30, 2020

Copy link
Copy Markdown

Thanks! I expected to be able to use relative_to() as described in this PR, I'm happy to know it is being worked on.

Python 3.9 included an is_relative_to() method to PurePath(). Should that method gain a strict argument as well?

domragusa and mueslo reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@domragusa

domragusa commented Oct 30, 2020 via email •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author
I don't know, a is_relative_to with that option would return always true because when you're allowed to traverse the tree structure every path is relative to the other (only exception being when one path is absolute and the other is not or if they are on different discs on windows) anyway if there are useful use cases that I'm missing let me know, I have no problem adding that functionality.
zeehio reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

terryjreedy
terryjreedy reviewed May 14, 2021
View reviewed changes
Comment thread Doc/library/pathlib.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@terryjreedy

terryjreedy commented May 14, 2021

Copy link
Copy Markdown
Member

A what's new entry should go in What's New 3.11.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

domragusa and others added 2 commits May 15, 2021 12:26
@domragusa
Merge branch 'python:main' into relative_to_strict_param
09422e7
@domragusa @terryjreedy
Update Doc/library/pathlib.rst
d9c07c2
Co-authored-by: Terry Jan Reedy <[email protected]>
@domragusa

domragusa commented May 15, 2021

Copy link
Copy Markdown
Contributor Author

Hi, I'm sorry but I'm not sure about the what's new entry, should I put it under Improved Modules > pathlib? Something like:

pathlib
-------

:meth:`pathlib.PurePath.relative_to` now allows a path that is not a direct
child of the current if the *strict* keyword-only argument is set to False,
the new behavior is more consistent with :meth:`os.relpath`.
(Contributed by Domenico Ragusa in :issue:`40358`.)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

barneygale
barneygale reviewed May 17, 2021
View reviewed changes

@barneygale barneygale left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - a couple of comments on the docs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Doc/library/pathlib.rst Outdated
Comment on lines +568 to +571
.. versionadded:: 3.11
The *strict* argument (pre-3.11 behavior is strict).
.. versionadded:: 3.10
The *strict* argument (pre-3.10 behavior is strict).

@barneygale barneygale May 17, 2021

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplication?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@domragusa domragusa May 17, 2021

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't notice this, thanks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Doc/library/pathlib.rst Outdated
ValueError: '/etc/passwd' is not on the same drive as 'foo' OR one path is relative and the other is absolute.

NOTE: This function is part of :class:`PurePath` and works with strings. It does not check or access the underlying file structure.
If the path doesn't start with *other* and *strict* is ``True``, :exc:`ValueError` is raised. If *strict* is ``False`` and one path is relative and the other is absolute or if they reference different drives :exc:`ValueError` is raised.

@barneygale barneygale May 17, 2021

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the key difference should be spelled out in terms of whether adding .. entries is allowed. How about something like this?

In strict mode (the default), the path must start with *other*. In non-strict 
mode, ``..`` entries may be added to form the relative path. In all other 
cases, such as the paths referencing different drives, :exe:`ValueError` is 
raised.

.. warning::
    Non-strict mode assumes that no symlinks are present in the path; you 
    should call :meth:`~Path.resolve` first to ensure this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@barneygale

barneygale commented May 17, 2021

Copy link
Copy Markdown
Contributor

Hi, I'm sorry but I'm not sure about the what's new entry, should I put it under Improved Modules > pathlib? Something like:

pathlib
-------

:meth:`pathlib.PurePath.relative_to` now allows a path that is not a direct
child of the current if the *strict* keyword-only argument is set to False,
the new behavior is more consistent with :meth:`os.relpath`.
(Contributed by Domenico Ragusa in :issue:`40358`.)

This is indeed tricky to phrase! How about:

:meth:`pathlib.PurePath.relative_to` now supports generating relative paths
containing ``..`` entries if its new *strict* keyword-only argument is set to
``False``. The new behavior is more consistent with :func:`os.path.relpath`.
(Contributed by Domenico Ragusa in :issue:`40358`.)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@domragusa
fix wording in documentation as requested
029c3d5
domragusa
domragusa commented May 17, 2021
View reviewed changes

@domragusa domragusa left a comment

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@barneygale I've added the what's new entry with the clearer language and fixed the documentation as you suggested. Thanks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Doc/library/pathlib.rst Outdated
Comment on lines +568 to +571
.. versionadded:: 3.11
The *strict* argument (pre-3.11 behavior is strict).
.. versionadded:: 3.10
The *strict* argument (pre-3.10 behavior is strict).

@domragusa domragusa May 17, 2021

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't notice this, thanks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@barneygale

barneygale commented May 17, 2021

Copy link
Copy Markdown
Contributor

I'm trying to think if there's a better word than 'strict'. In other places, 'strict' usually refers to our behaviour when we get an error from the OS. In this case it's more to do with whether we tolerate the possibility of the path's meaning changing in our pure operation that can't ever raise OSError anyway.

Perhaps something like parents=True to enable the new behaviour, mirroring mkdir()? Or walk_up?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@domragusa
fix typo
01e789b
@domragusa

domragusa commented May 18, 2021

Copy link
Copy Markdown
Contributor Author

Yeah, strict doesn't give any clue about the actual function. I like walk_up, I'll change it to that tomorrow.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@domragusa
change strict argument to walk_up in pathlib.PurePath.relative_to
39b24f3
@domragusa

domragusa commented May 29, 2021

Copy link
Copy Markdown
Contributor Author

Sorry for the long delay, I just updated the code to follow the suggestion you made about the name of the option.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

barneygale
barneygale reviewed Jun 24, 2021
View reviewed changes
Comment thread Doc/library/pathlib.rst Outdated
Comment on lines +566 to +569
When *walk_up* is False, the default, the path must start with *other*,
when it's True ``..`` entries may be added to form the relative path. In
all other cases, such as the paths referencing different drives,
:exc:`ValueError` is raised.

@barneygale barneygale Jun 24, 2021

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this documentation needs to occur before the examples of using walk_up.

So, either move it after "ValueError is raised", or split the examples into two parts.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

domragusa reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@barneygale

barneygale commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

Aside from a small docs issue, this looks great to me. Really useful feature, nice implementation :)

Others may want to chip in on the naming of walk_up.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@CAM-Gerlach

CAM-Gerlach commented Dec 10, 2021

Copy link
Copy Markdown
Member

Hey @domragusa , there has been further interest in this over on the Python discourse; any chance you could tweak the docs, fix the merge conflicts and retrigger the builds?

barneygale reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brettcannon brettcannon added the topic-pathlib label Jun 21, 2022
@domragusa domragusa mannequin mentioned this pull request Jun 21, 2022
pathlib's relative_to should behave like os.path.relpath #84538
Closed
@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
domragusa and others added 3 commits October 22, 2022 02:49
@domragusa
improve variable names
8a4edb5
@domragusa
remove trailing spaces
e6cf222
@domragusa
Merge branch 'main' into relative_to_strict_param
b8b218c
@domragusa

domragusa commented Oct 22, 2022

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot added awaiting change review and removed awaiting changes labels Oct 22, 2022
@bedevere-bot

bedevere-bot commented Oct 22, 2022

Copy link
Copy Markdown

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot
bedevere-bot requested a review from brettcannon October 22, 2022 02:11
brettcannon
brettcannon approved these changes Oct 28, 2022
View reviewed changes
Comment thread Doc/library/pathlib.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot added awaiting merge and removed awaiting change review labels Oct 28, 2022
brettcannon added 2 commits October 28, 2022 15:54
@brettcannon
Update Doc/library/pathlib.rst
a4c77c2
@brettcannon
Merge branch 'main' into relative_to_strict_param
97a5e22
@brettcannon brettcannon added the 🤖 automerge label Oct 28, 2022
@miss-islington

miss-islington commented Oct 28, 2022

Copy link
Copy Markdown
Contributor

@domragusa: Status check is done, and it's a failure or timed out ❌.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brettcannon brettcannon changed the title bpo-40358: add strict argument to pathlib.PurePath.relative_to gh-84538: add strict argument to pathlib.PurePath.relative_to Oct 28, 2022
@domragusa

domragusa commented Oct 28, 2022

Copy link
Copy Markdown
Contributor Author

I'm not sure I understand the last comment, what status check has failed? Should I do anything else?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington

miss-islington commented Oct 28, 2022

Copy link
Copy Markdown
Contributor

Status check is done, and it's a success ✅.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington
miss-islington merged commit e089f23 into python:main Oct 28, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Oct 28, 2022
@brettcannon

brettcannon commented Oct 28, 2022

Copy link
Copy Markdown
Member

@domragusa thanks so much for your patience with this!

domragusa and zeehio reacted with heart emoji
All reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@domragusa

domragusa commented Oct 28, 2022

Copy link
Copy Markdown
Contributor Author

I'm very happy to see this being merged, thanks to @brettcannon and everyone else who helped along the way :)

cmaureir, zeehio, and ctismer reacted with heart emoji
All reactions
  • ❤️ 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ctismer

ctismer commented Oct 29, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Maybe I‘m misunderstanding, but this was an incompatibility to path.relpath which is now fixed. Should it not be considered a bug, and the fix be back-ported to a few versions?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@domragusa

domragusa commented Oct 29, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

I don't think anybody is currently relying on pathlib for this behaviour, in fact when I looked around for anwsers they were generally saying to use os.path.relpath and also the original developer told me it was the intended behaviour (see #84538), so I'm not convinced we should consider it as a bug.

Anyway, if there are specific arguments for backporting it's fine by me.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ctismer

ctismer commented Oct 31, 2022

Copy link
Copy Markdown
Contributor

Ok, the table at the end of the pathlib doc was augmented, and relpath was listed with a comment.
You are right, it is not a bug. Still, we were trying to convert everything, but this is the only function that has no equivalent at os.path, and it would take quite long until this change makes it through our supported versions from 3.7 on :)

drts01 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@zeehio

zeehio commented Oct 31, 2022

Copy link
Copy Markdown

Hi

With respect to the backporting, I took this pull request and created a small package out of it, following the backports package guidelines.

I included some of the tests.

https://pypi.org/project/backports-pathlib-relative-to/0.1.0/

https://github.com/zeehio/backports-pathlib-relative-to

Issues and pull requests are welcome

domragusa and ctismer reacted with heart emoji
All reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@barneygale

barneygale commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

FYI, I've opened a bug and PR to address one final incompatibility between PurePath.relative_to() and os.path.relpath().

domragusa reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@h-mayorquin h-mayorquin mentioned this pull request Jul 17, 2023
Allow relative_to with recursive=False (in-place on objects) SpikeInterface/spikeinterface#1824
Closed
@dundargoc dundargoc mentioned this pull request Dec 30, 2024
feat: add vim.fs.relpath neovim/neovim#31790
Merged
@sholderbach sholderbach mentioned this pull request Apr 22, 2025
Allow path relative-to to use parent directory symbols nushell/nushell#14055
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@barneygale barneygale barneygale left review comments
@CuriousLearner CuriousLearner CuriousLearner left review comments
@CAM-Gerlach CAM-Gerlach CAM-Gerlach left review comments
@terryjreedy terryjreedy terryjreedy left review comments
@brettcannon brettcannon brettcannon approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic-pathlib

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

13 participants

@domragusa @zeehio @terryjreedy @barneygale @CAM-Gerlach @cmaureir @bedevere-bot @miss-islington @brettcannon @ctismer @CuriousLearner @the-knights-who-say-ni @ezio-melotti
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.