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

Enable pdf build of the docs #734

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Enable pdf build of the docs #734

wants to merge 1 commit into from

Conversation

glatterf42
Copy link
Member

@glatterf42 glatterf42 commented Aug 10, 2023

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.

@glatterf42 glatterf42 added enh New features & functionality docs Documentation labels Aug 10, 2023
@glatterf42 glatterf42 self-assigned this Aug 10, 2023
@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

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

❗ 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
Copy link
Member Author

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
Copy link
Member Author

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.

* Convert all \text{*_*} to \text{*\_*}
@glatterf42 glatterf42 mentioned this pull request Aug 11, 2023
1 task
khaeru pushed a commit that referenced this pull request Aug 14, 2023
See also discussion on #734
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation enh New features & functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant