-
Notifications
You must be signed in to change notification settings - Fork 22
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
Documentation site build rework #204
Documentation site build rework #204
Conversation
When using multiple custom Sphinx objects (one per builder, i.e. one each for linkcheck and html), a bunch of warnings get thrown when creating the second object due to all of the nodes/roles/directives already existing from the first object. There's not a good way around this in Sphinx, but I can't think of an instance where these warnings would be called in normal docs building, so we can suppress them for now to get this working in the CI
|
||
The documentation uses the Jupyter notebook tutorials in the `examples` directory. | ||
When building the documentation locally you will need to have installed [pandoc](https://pandoc.org/installing.html) and [gifsicle](https://github.com/kohler/gifsicle). | ||
We recommend installing pandoc using its Anaconda distribution: `conda install -c conda-forge pandoc`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this while investigating #203. Pandoc is a pain to get playing nice with Python on Windows using the methods suggested on the website, but there is an undocumented precompiled version supported on conda-forge that works like a charm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems to be the case with a lot of codes that require compiling.
def linkcheck(): | ||
app = Sphinx(source_dir, | ||
conf_dir, | ||
build_dir, | ||
doctree_dir, | ||
'linkcheck', | ||
warningiserror=True) | ||
app.build() | ||
|
||
|
||
def html(): | ||
app = Sphinx(source_dir, | ||
conf_dir, | ||
build_dir, | ||
doctree_dir, | ||
'html', | ||
warningiserror=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think writing it like this is redundant, but Sphinx doesn't have a good way of handling multiple builders outside of the Makefile interface at the moment. However, this should be addressed in sphinx-doc/sphinx#5618 whenever it gets merged.
|
||
linkcheck_ignore = [ | ||
'https://github.com/HIPS/autograd/blob/master/docs/tutorial.md#supported-and-unsupported-parts-of-numpyscipy', | ||
'https://doi.org/10.2172/1330189', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out we were getting a warning for this URL in linkcheck
that was naturally never caught in the CI. See urllib3/urllib3#2653 (comment) for an identical warning and explanation for why this happens. I think we can safely ignore this warning -- the SSLError at the root of this problem should supposedly be solved in Python 3.12 anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! 👏 :)
@@ -66,15 +66,15 @@ The homepage source code is in `./docs/source/index.rst`. | |||
To build the documentation locally (not required but good check) | |||
|
|||
```bash | |||
cd docs | |||
make html | |||
python3 docs/build_docs.py | |||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the make html/pdf/etc
and make clean
are common interfaces. It is a shame to have to replace them. But that's OK, it is not like we use this often. It is mostly just the CI.
|
||
The documentation uses the Jupyter notebook tutorials in the `examples` directory. | ||
When building the documentation locally you will need to have installed [pandoc](https://pandoc.org/installing.html) and [gifsicle](https://github.com/kohler/gifsicle). | ||
We recommend installing pandoc using its Anaconda distribution: `conda install -c conda-forge pandoc`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems to be the case with a lot of codes that require compiling.
Description
WecOptTool uses Sphinx to build documentation via a Makefile, but Sphinx handles exceptions with internal try/except clauses that do not return error codes to higher level programs, see https://stackoverflow.com/a/58398714. This prevents Sphinx build errors from being returned to the GitHub Actions CI in the WecOptTool release workflow, which can cause website build failures without any notification. This PR replaces the Sphinx build Makefile with
build_docs.py
, which interfaces with Sphinx objects directly, allowing Sphinx build exceptions to propagate to the CI correctly. The Contributing documentation is also updated accordingly.Fixes #193. Since everything regarding Sphinx is now in pure Python and not dependent on the
make
command, this also conveniently fixes #203.Type of PR
Checklist for PR
Additional details
I tried to translate everything from the old Makefile into either
build_docs.py
orclean_docs.py
, but I'm no expert inmake
commands. If there's any functionality being lost, let me know and I can add it to this PR.