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

Fix compatibility with Sphinx 8 #199

Merged

Conversation

agriyakhetarpal
Copy link
Collaborator

@agriyakhetarpal agriyakhetarpal commented Aug 5, 2024

Description

The function _process_docstring_examples used app.env.doc2path, which now needs paths instead of strings, since Sphinx 9 will drop support for representing paths as strings, which was raising warnings for the new release of Sphinx, i.e., version 8. This uses pathlib.Path() to process the path. A few other type hints have been added to the function's arguments.

Closes #197

Additional context

xref docs build failures in SciPy downstream: scipy/scipy#21323, scipy/scipy#21324

@agriyakhetarpal
Copy link
Collaborator Author

Thanks for flagging the issue, @WarrenWeckesser! This is ready for review.

@steppi steppi added the bug Something isn't working label Aug 5, 2024
@agriyakhetarpal agriyakhetarpal changed the title Compatibility with Sphinx 8 Fix compatibility with Sphinx 8 Aug 5, 2024
@WarrenWeckesser
Copy link

Thanks for the quick fix, @agriyakhetarpal. When I locally replaced jupyterlite_sphinx 0.16.3 with this PR and built the SciPy docs, I no longer got the error from the jupiterlite_sphinx code. I still got an error, but now it is coming from myst_nb. So I suspect this fix exposes another package that needs to be updated.

@steppi
Copy link
Collaborator

steppi commented Aug 5, 2024

Looking into it a little. I think this is actually a Sphinx bug which has been fixed at least in main. It would be weird if Sphinx wouldn't accept it's own output from doc2path, and if you look at the latest docs, it now returns a _StrPath, which is a pathlib path which has string methods.

Nevermind, I understand the issue now. It's that we were treating the output as a string, _StrPath has string methods, but they are deprecated, and eventually it will just return a pathlib path I guess. Sorry for the noise.

@WarrenWeckesser
Copy link

FYI: I created an issue at myst_nb: executablebooks/MyST-NB#619. As I explain there, I can get the SciPy docs to build if I apply the same fix as used here to one line in the myst_nb code.

@agriyakhetarpal
Copy link
Collaborator Author

Thanks for trying this out with MyST-NB, @WarrenWeckesser! I think we should be good to go with this PR, then, @steppi – since the warning is no longer emitted here. The MyST-NB one could be suppressed if the fix there takes longer to land.

@agriyakhetarpal agriyakhetarpal requested a review from steppi August 6, 2024 12:19
Copy link
Collaborator

@steppi steppi left a comment

Choose a reason for hiding this comment

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

lgtm

@steppi steppi merged commit 84537b0 into jupyterlite:main Aug 6, 2024
5 checks passed
@agriyakhetarpal agriyakhetarpal deleted the compatibility-with-sphinx-8 branch August 6, 2024 12:58
@Carreau Carreau added this to the 0.16.4 milestone Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compatibility with Sphinx 8?
4 participants