Closed fohrloop closed 5 months ago
Holoviews docs build process before and after the fix. The fix takes 108 warnings away from the logs, so it would be most likely fixing 108 broken links in https://holoviews.org/
niko@niko-ubuntu-home:~/tmp$ cat holoviews-logs.txt | grep "WARNING: 'myst' reference target not found" | grep -v "not found: #" | wc -l
155
niko@niko-ubuntu-home:~/tmp$ cat holoviews-logs-nbsite-updated.txt | grep "WARNING: 'myst' reference target not found" | grep -v "not found: #" | wc -l
47
The failing test tests / Documentation (pull_request)
is expected as you don't have the permissions.
A problem we have with the link conversion is that the links in a notebook environment are case-insensitive, and the URL on the webpage is not. Or at least that is how I remember it...
Do you mean that a link in the notebook 03-Applying_Customizations.ipynb
might for some reason be (notice small c)
[customization](../getting_started/2-customization.ipynb)
when file structure is
📁 doc/
├─📁 user_guide/
│ ├─📄 Applying_Customizations.rst
│ └─📄 Style_Mapping.rst
└─📁 getting_started/
└─📄 Customization.rst
📁 examples/
├─📁 user_guide/
│ ├─📄 03-Applying_Customizations.ipynb
│ └─📄 04-Style_Mapping.ipynb
└─📁 getting_started/
└─📄 2-Customization.ipynb
and that would work fine with notebooks? But the created link in the sphinx doctree should be
[customization](../getting_started/Customization.rst)
?
If so, the doc build process should emit warning for the missing target. Which in ideal case would be seen and fixed. Another way to fix that would be to edit the FixNotebookLinks._get_sourcefile
and do case-insensitive glob search, instead of relying on a fixed list formed by FixNotebookLinks._iter_source_file_candidates
.
That's perhaps out of scope of this PR, though?
That's perhaps out of scope of this PR, though?
Yes. It was more of a heads-up.
I really appreciate the efforts and attention to detail that have gone into this PR. I will merge and cut a dev release.
This PR attempts to fix
What was done?
The "notebook" directive uses a subclass of nbconvert.preprocessors.PreProcessor called FixNotebookLinks to fix links in Jupyter Notebooks markdown cells. For example
should be converted to
..if a Customization.rst source file (the file with notebook directive) exists in the ../getting_started/ directory. But if the link target notebook is not in the same directory as the linking notebook, the current version won't create a link, and during sphinx-build myst warns about a missing reference target like this:
I'm not completely sure why the first type of warnings were emitted (reference target is .ipynb), but the second type of warning (reference target is .rst file) occurred because in the old implementation the "target" in
[some text](target)
was replaced completely by the found source (.rst or .md) file. That means that with file structureand
in
examples/user_guide/03-Applying_Customizations.ipynb
one would getin the doctree for sphinx, instead of
In addition to this, any anchors like #spam in
[customization](../getting_started/2-Customization.ipynb#spam)
would be removed.This PR fixes those issues by making a
to be
depending on which of the source files (.rst/.md) is found first, if any.
How to test this?
Testing this is a bit difficult as explained here, but you may install the "bottom-up MWE" from https://github.com/fohrloop/holoviews-issue-6086-mwe and build the docs with
Or, after first build (when /docs has the .ipynb files), with
Without these changes
With the current nbsite version, one would expect to see a broken link
and following warning during build process:
With these changes
One should see:
and the build process output should not have the abovementioned reference target not found warning.
About the code
nbsite/nbbuild.py
is not formatted using any tools. The test module is formatted using black. Please instruct how you would like the code to be formatted.