Skip to content

Conversation

@diegoe
Copy link
Contributor

@diegoediegoe commented Mar 17, 2021

Add a warning describing the behavior of Path objects when combining a
Path object with an anchored-path string, using the / operator.

https://bugs.python.org/issue43525

Add a warning describing the behavior of Path objects when combining a Path object with an anchored-path string, using the `/` operator.
>>> p / '/bin'
PurePosixPath('/bin')
>>> p / '/bin' / '/python'
PurePosixPath('/python')
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Is this ok? I didn't want to use python3 to not make the example susceptible to bit-rot 😆

I couldn't think of another executable. Maybe a different path would be a better example?

Maybe I'm overthinking it! 😆 🤣

PurePosixPath('/usr/bin')

Note that a string containing a leading '`/`' character will be considered
an absolute path, making the last to be taken as the anchor for the new
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

"last"? "last one"? The phrasing is copied from pathlib.PurePath:

When several absolute paths are given, the last is taken as an anchor (mimicking os.path.join()’s behaviour):

@barneygale
Copy link
Contributor

This behaviour is already explained a few paragraphs before.

@diegoe
Copy link
ContributorAuthor

This behaviour is already explained a few paragraphs before.

The way I ran into this caveat was that I replaced naive string concatenations on an old codebase, and changed stuff like this:

filepath=ROOT_DIR+"/some_file.txt"

Into:

root=Path(...) filepath=root/"/some_file.txt"

I missed the leading slash on a few updates, and got the confusing result of filepath "ignoring" root when composing the path with the / operator.

I went to read the docs on the / operator and didn't see the absolute path caveat mentioned, so I thought it might be worth repeating.

Perhaps my suggestion is too verbose? I think it would be good enough without the example. Feel free to close if you don't think the change is worth it at all.

Copy link
Contributor

@hauntsaninjahauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I agree that this is too verbose, given that it's been explained a couple paragraphs above.

I'm still sympathetic to the fact that this can be surprising. I opened a different PR that just adds an extra example over here: #100737

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

Labels

awaiting reviewdocsDocumentation in the Doc dirskip newstopic-pathlib

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@diegoe@barneygale@hauntsaninja@brettcannon@the-knights-who-say-ni@ezio-melotti@bedevere-bot@AlexWaygood