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

Support string methods on path objects #11619

Merged
merged 3 commits into from Aug 23, 2023
Merged

Conversation

AA-Turner
Copy link
Member

cc: @stephenfin @tronical @picnixz @lucyleeow
xref: #11605

This PR would allow using str methods on paths (e.g. app.outdir.rstrip(...)). It is however very fragile.

The alternative would be to entirely revert pathlib support, but I fear downstream may have already adapted to expect usage of pathlib.Path objects.

Thoughts appreciated.

A

@AA-Turner AA-Turner linked an issue Aug 18, 2023 that may be closed by this pull request
@picnixz
Copy link
Member

picnixz commented Aug 18, 2023

I personally think that path objects should be used whenever possible. IIRC there has been some work in CPython to improve pathlib performances, e.g., by implementing some operations in C.

As such, it sould be better in the long term to switch now and inform users as much as possible that treating them as strings is deprecated.

Since we made the changes and some upstream repos may have already switched, it's maybe better not to revert (if we eventually plan to completely drop str aupport in some future version).

Copy link
Contributor

@stephenfin stephenfin left a comment

Choose a reason for hiding this comment

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

This is a total hack and I approve 👍 Let's get this out and drop it in 8.x as you suggest

@AA-Turner AA-Turner merged commit 6b17dd1 into sphinx-doc:master Aug 23, 2023
27 checks passed
@AA-Turner AA-Turner deleted the str-path branch August 23, 2023 18:33
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sphinx 7.2.0 problems with pathlib.Path instead of string paths
3 participants