Enable pdf build of the docs

glatterf42 commented 1 year ago

Context

This PR was created to enable the PDF builds of the docs again since them failing had kept the latest version of the docs from being uploaded to RTD. However, as detailed in the comments below, I ran into more severe issues that would require significant time to fix. Thus, for the time being, we decided to disable the PDF builds with #735 so that the latest version of the docs passes again. If providing a PDF version of the docs becomes higher priority in the future, this PR might include valuable information for further attempts.

Aim

While the docs seem to build fine for epub and html format, they do not for the pdf format, keeping the whole process from succeeding and, thus, a new version from being uploaded. Through comparison with ixmp, I think the issue is the specification of lualatex as the latex engine for building the pdf; at least locally, the latexmkrc file looked like the one for ixmp after commenting this out. This PR therefore removes the specification from doc/conf.py.

How to review

Check that the documentation build on RTD succeeds.
Read the diff and note that the CI checks all pass.

PR checklist

[ ] Continuous integration checks all ✅
~[ ] Add or expand tests; coverage checks both ✅~ Only docs update.
~[ ] Add, expand, or update documentation.~ Only docs update.
~[ ] Update release notes.~ Only docs update.

codecov[bot] commented 1 year ago

Codecov Report

Merging #734 (c301191) into main (e748b8c) will not change coverage. The diff coverage is n/a.

:exclamation: Current head c301191 differs from pull request most recent head 0ed352a. Consider uploading reports for the commit 0ed352a to get more accurate results

@@          Coverage Diff          @@
##            main    #734   +/-   ##
=====================================
  Coverage   94.5%   94.5%           
=====================================
  Files         43      43           
  Lines       3463    3463           
=====================================
  Hits        3273    3273           
  Misses       190     190

glatterf42 commented 1 year ago

Aborted the tests since the docs were not building properly. There were several hundred latex errors of the sort

! Missing } inserted.
<inserted text> 
}
l.3771 ...y}}{\text{duration_period}_{y}}\end{split}

So I tried locally to fix this as in normal latex, this should probably be \text{duration\_period}. However, I then remembered that there is a conflict in the ways Mathjax renders latex math for html and latex does it itself. I remember trying to figure this out some months ago, but I didn't find a solution because writing the variable as above produces a literal '\' in html despite '_' being designated a special character in textmacros. I think we are using this package already, but can't quickly find where we load it (the way in which this should be done is detailed on the same page I linked above).

A quick solution that should succeed in updating the docs pages is to disable the pdf build, which I will do for now.

glatterf42 commented 1 year ago

Removing the pdf build will allow the docs to build and should thus be done in a separate PR. However, due to its name, it should not be done here.

One piece of good news I found today: we were, in fact, not using the textmacros package, but a few simple lines in doc/conf.py enable it. Hence, I converted all \text{*_*} to \text{*\_*}. However, building the pdf with latex still fails for a few reasons, of which I don't understand all in detail (yet), since there already is one that warrants clarification: Sphinx' latex builder only supports pdf, png, and jpeg formats, while we use webp and svg as well (and jpg, but that doesn't seem to be a problem). Getting this to work would require converting these images to supported types. I'm no expert in image formats, but I think some of them feature better resolution, smaller size, etc, so would we even want to convert these images to compatible types? If not, we might have to put the pdf versions of the docs to rest.

iiasa / message_ix