Skip to content

Support string methods on path objects#11619

Merged
AA-Turner merged 3 commits intosphinx-doc:masterfrom
AA-Turner:str-path
Aug 23, 2023
Merged

Support string methods on path objects#11619
AA-Turner merged 3 commits intosphinx-doc:masterfrom
AA-Turner:str-path

Conversation

@AA-Turner
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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