Skip to content

Conversation

@hauntsaninja
Copy link
Contributor

@hauntsaninjahauntsaninja commented Jan 5, 2023

This is feedback from #100737 (comment)

This matches the wording from the os.path.join docs better: https://docs.python.org/3/library/os.path.html#os.path.join

In particular, the previous use of "anchor" was incorrect given the pathlib definition of "anchor".

Co-authored-by: barneygale

This is feedback from python#100737 (comment) This matches the wording from the `os.path.join` docs better: https://docs.python.org/3/library/os.path.html#os.path.join In particular, the previous use of "anchor" was incorrect given the pathlib definition of "anchor". While matching wording, I noticed that the constructor section uses the word "segment". This word does not appear elsewhere in the docs or code; we already have "part" and "component" to refer to the same concept in the pathlib context.
Copy link
Member

@JelleZijlstraJelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good with some small nits; feel free to merge if @barneygale is on board.


However, in a Windows path, changing the local root doesn't discard the
previous drive setting::
On Windows, the drive letter is not reset when a drive-less absolute path
Copy link
Member

Choose a reason for hiding this comment

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

Is the drive necessarily one letter?

Copy link
ContributorAuthor

@hauntsaninjahauntsaninjaJan 6, 2023

Choose a reason for hiding this comment

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

Good question, I think the drive can be a UNC path. This also would need to get fixed in the os.path.join documentation: https://docs.python.org/3/library/os.path.html#os.path.join

>>> PureWindowsPath("hello", "//host/computer/dir", "/asdf") PureWindowsPath('//host/computer/asdf') >>> ntpath.join("hello", "//host/computer/dir", "/asdf") '//host/computer/asdf'

I'll update to "drive" and open a second PR for os.path.join if @barneygale concurs

Copy link
Contributor

Choose a reason for hiding this comment

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

"drive" makes more sense to me too.

@barneygale
Copy link
Contributor

While matching wording, I noticed that the constructor section uses the word "segment". This word does not appear elsewhere in the docs or code; we already have "part" and "component" to refer to the same concept in the pathlib context.

They're not quite the same! A "part" is a component in a fully-normalized path. Unless it's an anchor, a part can't contain any path separators. Contrast this with a "segment" which can contain separators and might even be a fully-fledged path in its own right!

@hauntsaninjahauntsaninja changed the title gh-87691: clarify use of anchor and segment in pathlib docsgh-87691: clarify use of anchor in pathlib docsJan 6, 2023
@hauntsaninja
Copy link
ContributorAuthor

hauntsaninja commented Jan 6, 2023

re segment: Ah, makes sense, I knew I had to be missing something

I filed #100783 for some issues in os.path.join, including "drive" vs "drive letter"

Co-authored-by: Barney Gale <barney.gale@gmail.com>
Co-authored-by: Barney Gale <barney.gale@gmail.com>
@hauntsaninjahauntsaninja merged commit 2f2fa03 into python:mainJan 6, 2023
@miss-islington
Copy link
Contributor

Thanks @hauntsaninja for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-100786 is a backport of this pull request to the 3.11 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.11 only security fixes label Jan 6, 2023
@bedevere-bot
Copy link

GH-100787 is a backport of this pull request to the 3.10 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.10 only security fixes label Jan 6, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 6, 2023
This is feedback from https://github.com/python/cpython/pull/100737GH-discussion_r1062968696 This matches the wording from the `os.path.join` docs better: https://docs.python.org/3/library/os.path.htmlGH-os.path.join In particular, the previous use of "anchor" was incorrect given the pathlib definition of "anchor". (cherry picked from commit 2f2fa03) Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Co-authored-by: Barney Gale <barney.gale@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 6, 2023
This is feedback from https://github.com/python/cpython/pull/100737GH-discussion_r1062968696 This matches the wording from the `os.path.join` docs better: https://docs.python.org/3/library/os.path.htmlGH-os.path.join In particular, the previous use of "anchor" was incorrect given the pathlib definition of "anchor". (cherry picked from commit 2f2fa03) Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Co-authored-by: Barney Gale <barney.gale@gmail.com>
miss-islington added a commit that referenced this pull request Jan 6, 2023
This is feedback from https://github.com/python/cpython/pull/100737GH-discussion_r1062968696 This matches the wording from the `os.path.join` docs better: https://docs.python.org/3/library/os.path.htmlGH-os.path.join In particular, the previous use of "anchor" was incorrect given the pathlib definition of "anchor". (cherry picked from commit 2f2fa03) Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Co-authored-by: Barney Gale <barney.gale@gmail.com>
miss-islington added a commit that referenced this pull request Jan 6, 2023
This is feedback from https://github.com/python/cpython/pull/100737GH-discussion_r1062968696 This matches the wording from the `os.path.join` docs better: https://docs.python.org/3/library/os.path.htmlGH-os.path.join In particular, the previous use of "anchor" was incorrect given the pathlib definition of "anchor". (cherry picked from commit 2f2fa03) Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Co-authored-by: Barney Gale <barney.gale@gmail.com>
@AlexWaygood
Copy link
Member

@barneygale, fancy adding yourself to CODEOWNERS so you get automatically requested for review on PRs touching pathlib? Now that you're a triager (and following the changes made to triagers' permissions relating to python/core-workflow#460), you can.

@AlexWaygood
Copy link
Member

You could also consider adding yourself to https://devguide.python.org/core-developers/experts/index.html#experts as a pathlib expert — there's already at least one triager on the experts list :-)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docsDocumentation in the Doc dirskip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@hauntsaninja@barneygale@miss-islington@bedevere-bot@AlexWaygood@JelleZijlstra