Skip to content

GH-128520: pathlib ABCs: add JoinablePath.__vfspath__() #133437

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

Merged
merged 3 commits into from
May 12, 2025

Conversation

barneygale
Copy link
Contributor

@barneygale barneygale commented May 5, 2025

In the abstract interface of JoinablePath, replace __str__() with __vfspath__(). This frees user implementations of JoinablePath to implement __str__() however they like (or not at all.)

Also add pathlib._os.vfspath(), which calls __fspath__() or __vfspath__(). This will be exported as a public function in the pathlib-abc PyPI package.

Some related discussion here: https://discuss.python.org/t/protocol-for-virtual-filesystem-paths/82753

In the abstract interface of `JoinablePath`, replace `__str__()` with
`__vfspath__()`. This frees user implementations of `JoinablePath` to
implement `__str__()` however they like (or not at all.)

Also add `pathlib._os.vfspath()`, which calls `__fspath__()` and/or
`__vfspath__()`.
Copy link
Member

@jaraco jaraco left a comment

Choose a reason for hiding this comment

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

This sounds like the right solution for decoupling the use of __str__ for casual user use and for the specific API purpose required by JoinablePath. I don't love the name vfspath. If that's the phrasing, I like to see at least vfs_path (separate the two terms). Even better would be to come up with a name that's a single word (__moniker__, __location__, __traversal__, ...), but I'm not coming up right now with any word I love. Regardless, I'll be happy with whatever property name you settle on.

@barneygale
Copy link
Contributor Author

Just in case it wasn't obvious, it's aping os.fspath() and __fspath__() by sticking a 'v' in front.

That said, I don't love it either! :P

@jaraco
Copy link
Member

jaraco commented May 7, 2025

Just in case it wasn't obvious, it's aping os.fspath() and __fspath__() by sticking a 'v' in front.

In that case, perhaps vfspath is better than vfs_path (for consistency). It's too late to consider __fs_path__ for clarity ;).

@barneygale
Copy link
Contributor Author

barneygale commented May 7, 2025

I guess virtual_path() and __virtual_path__() could work? Might be confused with __path__ though

@barneygale
Copy link
Contributor Author

Thanks for the review btw, much appreciated! I'll wait to see what Petr thinks before I go any further. I should add Mr Moore too, actually

@barneygale barneygale requested a review from pfmoore May 9, 2025 18:52
@pfmoore
Copy link
Member

pfmoore commented May 9, 2025

"Mr. Moore" - how grand 🙂 (If it's because you didn't know my first name, it's Paul 👋).

This seems like a reasonable enough change, and I agree with the basic idea that we should separate "the string representation" from "the representation as a path string". It's a very subtle distinction, though, and I think that before we can expect users to work with the new API, it'll need some pretty good documentation. I'm happy to defer that for now, but I don't think we can say user-defined path subclasses are ready for general use without it.

In practical terms, this feels like a small step, with a lot of compatibility and design questions to follow, but I'm fine with a small starting step.

@encukou
Copy link
Member

encukou commented May 12, 2025

Looks good on a cursory look.
I don't think I'll get to do an in-depth review soon.

@barneygale
Copy link
Contributor Author

Thanks for the sense check!

@barneygale barneygale merged commit 5dbd27d into python:main May 12, 2025
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants