search

jupyterlite / jupyterlite-sphinx

Sphinx extension using JupyterLite to render Notebooks
https://jupyterlite-sphinx.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
68 stars 21 forks source link

Fix compatibility with Sphinx 8 #199

Closed agriyakhetarpal closed 3 months ago

agriyakhetarpal commented 3 months ago

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: https://github.com/scipy/scipy/issues/21323, https://github.com/scipy/scipy/pull/21324

agriyakhetarpal commented 3 months ago

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

WarrenWeckesser commented 3 months ago

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 commented 3 months ago

~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 commented 3 months ago

FYI: I created an issue at myst_nb: https://github.com/executablebooks/MyST-NB/issues/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 commented 3 months ago

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.