Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-87691: add an absolute path pathlib example in / operator docs #100737

Merged
merged 3 commits into from
Jan 5, 2023

Conversation

hauntsaninja
Copy link
Contributor

@hauntsaninja hauntsaninja commented Jan 4, 2023

The behaviour is fully explained a couple paragraphs above, but it may be useful to have a brief example to cover the behaviour.

Automerge-Triggered-By: GH:hauntsaninja

The behaviour is fully explained a couple paragraphs above, but it may
be useful to have a brief example to cover the behaviour.
@hauntsaninja
Copy link
Contributor Author

hauntsaninja commented Jan 4, 2023

This is meant as a replacement for #24900. If we feel this is still unnecessary, we can just close out the issue.

cc @barneygale

@hauntsaninja hauntsaninja changed the title gh-87691: provide an absolute pathlib example under operator gh-87691: add an absolute path pathlib example in / operator docs Jan 4, 2023
Doc/library/pathlib.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@barneygale barneygale 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 to me!

@JelleZijlstra
Copy link
Member

Where is this behavior explained? I don't see it in https://docs.python.org/3.10/library/pathlib.html#operators. I think a sentence spelling out the behavior might be useful.

@hauntsaninja
Copy link
Contributor Author

It's explained in https://docs.python.org/3.10/library/pathlib.html#pathlib.PurePath , which is a couple paragraphs before Operators. Search for "When several absolute paths are given, the last is taken as an anchor"

@JelleZijlstra
Copy link
Member

That's about the Path constructor, not the slash operator. I don't think it's obvious to users that these behave the same.

@hauntsaninja
Copy link
Contributor Author

Hmm, what would you recommend?

We could copy over just the text or the text and the examples. Also note the Windows drive example, which isn't covered by https://github.com/python/cpython/pull/24900/files

Something like:

The slash operator helps create child paths, mimicking os.path.join()’s behaviour. For instance, when several absolute paths are given, the last is taken as an anchor; for a Windows path, changing the local root doesn’t discard the previous drive setting

@JelleZijlstra
Copy link
Member

That sounds good to me.

@miss-islington
Copy link
Contributor

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

@miss-islington miss-islington merged commit 1ae619c into python:main Jan 5, 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-100780 is a backport of this pull request to the 3.11 branch.

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

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 5, 2023
…cs (pythonGH-100737)

The behaviour is fully explained a couple paragraphs above, but it may be useful to have a brief example to cover the behaviour.
(cherry picked from commit 1ae619c)

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
Automerge-Triggered-By: GH:hauntsaninja
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jan 5, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 5, 2023
…cs (pythonGH-100737)

The behaviour is fully explained a couple paragraphs above, but it may be useful to have a brief example to cover the behaviour.
(cherry picked from commit 1ae619c)

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
Automerge-Triggered-By: GH:hauntsaninja
@hauntsaninja hauntsaninja deleted the gh-87691 branch January 5, 2023 22:59
miss-islington added a commit that referenced this pull request Jan 5, 2023
…-100737)

The behaviour is fully explained a couple paragraphs above, but it may be useful to have a brief example to cover the behaviour.
(cherry picked from commit 1ae619c)

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
Automerge-Triggered-By: GH:hauntsaninja
miss-islington added a commit that referenced this pull request Jan 5, 2023
…-100737)

The behaviour is fully explained a couple paragraphs above, but it may be useful to have a brief example to cover the behaviour.
(cherry picked from commit 1ae619c)

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
Automerge-Triggered-By: GH:hauntsaninja
@JelleZijlstra
Copy link
Member

@barneygale sorry this got merged without your review, I forgot about the automerge label when I approved.

@barneygale
Copy link
Contributor

D'oh, I should have noticed. My feedback is just nits, the patch that went in looks fine.

@hauntsaninja
Copy link
Contributor Author

Hmm, I think your point that "anchor" is defined differently in pathlib is more than just a nit. I'll open another PR, sigh

And sorry for my part in the automerge situation!

hauntsaninja added a commit to hauntsaninja/cpython that referenced this pull request Jan 5, 2023
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.
hauntsaninja added a commit that referenced this pull request Jan 6, 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: Barney Gale <barney.gale@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news topic-pathlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants