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

Build docs better in CI/CD #132

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Build docs better in CI/CD #132

wants to merge 14 commits into from

Conversation

CasperWA
Copy link
Collaborator

@CasperWA CasperWA commented Mar 19, 2024

This change does away with the outdated and non-maintained GitHub action ammaraskar/sphinx-action. It is using an old version of the underlying Docker image sphinxdoc/sphinx (see also this source), which enforced everything running in Python 3.8 and not the actual Python version specified in the workflow here (Python 3.10).

The new way of building the documentation has been tested locally, and the CI here also seems to work as intended, so it should be good 👍
Instead of using the GH Actions, it simply utilizes the Makefile that exists in the sphinx folder. So we still install pandoc using APT and then from within the working directory sphinx we run make html, which will generate the documentation under sphinx/_build/html, which is the same as was done previously, i.e., the upload step does not need to change.

Finally, I've updated the versions of all dependencies where possible - testing everything locally.
The only package I've kept at the current version in pydata-sphinx-theme, as updating this to the latest version (v0.15.2 as of writing) resulted in a severe change in the look and feel of the documentation. Extra time, effort, and care should be taken when updating this package version at a later stage.

Note, also packaging has not been updated to v24 due to a version incompatibility with the latest version of EMMOntoPy.
Edit: Already done by @dependabot now.


To see the built documentation do either of the following:

Unpack and view the built artifact:

  • Go to the latest run of the build documentation workflow for this branch and download the documentationHTML artifact at the bottom of the "Summary" page (as of writing, this is the link to the artifact).
  • This will download a zip-file to your system.
  • Unpack it and find and open the index.html file.

Build the documentation locally and check it out:

  • git clone this repository locally, preferably when running a virtual environment (see e.g., some documentation on venv and virtualenv).
  • git checkout the branch cwa/fix-docs-building.
  • From the current directory of the (possibly freshly cloned) repository, run the following commands in your shell/terminal:
    python -m pip install -U pip
    pip install -U pip setuptools wheel
    pip install -r requirements_docs.txt -r sphinx/requirements.txt
  • Note, you need pandoc installed on you system. If you are running on Linux (natively or through WSL) this is as simple as installing it from your package manager of choice. E.g., using APT:
    sudo apt-get update && sudo apt-get install -y pandoc
    For more information about installing pandoc, see their documentation.
  • Finally, if there have been no issues so far, you should be able to run the following commands to build the documentation as intended:
    python sphinx/ttl_to_rst.py
    cd sphinx
    make html
  • Go to the sphinx/_build/html folder and open the index.html file in your browser of choice.

One should especially focus on the Sphinx update (from v5 to v7), rdflib
update (from v6 to v7) and the necessity to keep pydata-sphinx-theme at
its current version due to too many differences in the look and feel of
the docs if updated to the latest version.

Also, all currently unversioned dependencies have been fixed to a
version.
Avoid using the - quite outdated - ammaraskar/sphinx-action GitHub
Action. It was forcefully using Python 3.8 due to using a very old
version of the underlying docker image sphinx-doc/sphinx.
@jsimonclark jsimonclark requested a review from xavierr April 1, 2024 17:25
@jsimonclark
Copy link
Collaborator

jsimonclark commented Apr 1, 2024

@xavierr , could you please have a look at this and see what you think? If it's a good idea, we should consider updating our other CI/CD workflows with sphinx.

Support installing all requirements files at once (resolving that there
are no sub-dependency issues).
All warnings have been removed by:
- Move `!pip install` statements to a separate cell in all notebooks,
  making the cell "shellscript" code and add a sentence immediately
  before stating the same thing.
- Add all files to a Sphinx `toctree` tag, specifically extending the
  header nav with the "More" dropdown, which includes "contribute",
  "tools", and "resources".
- Find online and add the image for the Protégé configuration.

Update the configuration to remove the primary sidebar from pages that
have no section navigation.

Since no warnings are produced (locally) now, the CI workflow has been
updated to include the `-W` flag for sphinx-build, which treats warnings
as errors during the build.
@CasperWA
Copy link
Collaborator Author

Updated this further to remove all Sphinx build warnings. In this way, the -W flag can also be added to the CI workflow, which will treat all warnings as errors, meaning if warnings are emitted in the future due to some changes, it will make the CI workflow fail. This is intended to ensure warnings are considered seriously by any contributor, making them consider whether the warning is something they should react on or is something that can be ignored (in which case it should manually be ignored from the configuration).

Note, to temporarily avoid having the documentation build fail on warnings (which are in their essence not critical), the -W flag can be removed from the SPHINXOPTS environment variable (see the relevant CI workflow):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants