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

Documentation: Investigate warnings in the PDF documentation generation process #2093

Closed
6 of 21 tasks
JohnHalleyGotway opened this issue Mar 16, 2023 · 24 comments
Closed
6 of 21 tasks
Assignees
Labels
alert: NEED ACCOUNT KEY Need to assign an account key to this issue component: documentation Documentation issue priority: medium Medium Priority reporting: DTC NCAR Base NCAR Base DTC Project reporting: DTC NOAA BASE NOAA Office of Atmospheric Research DTC Project type: enhancement Improve something that it is currently doing
Milestone

Comments

@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Mar 16, 2023

Replace italics below with details for this issue.

Describe the Enhancement

While working on #2087, @JohnHalleyGotway noted that 11,440 warnings are produced while generating the PDF version of the METplus documentation. They can be broken down as follows:

  • 6000: Package fancyhdr Warning: \headheight is too small (12.0pt):
  • 4686: LaTeX Warning: Hyper reference
  • 712: LaTeX Warning: Reference
  • 42: others

This issue is to investigate these warnings further and resolve as many as are easily resolved. Check to see if these warnings actually result in undesirable outcomes in the rendered PDF, such as content being cut off on the right side of the page. If so, prioritize fixing those issues first.

Recommend starting by building the PDF documentation locally on a laptop and counting up the number of warning messages:

conda activate /home/met_test/.conda/envs/metplus_dev.v5.1
cd docs
make clean latexpdf | tee make_docs_pdf.log
egrep -i "warning" make_docs_pdf.log | wc -l

Time Estimate

Unknown

Sub-Issues

Consider breaking the enhancement down into sub-issues.
None needed.

Relevant Deadlines

List relevant project deadlines here or state NONE.

Funding Source

Split across any of the keys you have allocation on (only charge the numbers below if you have an allocation for that number)
2702691
2773542
2792542

Define the Metadata

Assignee

  • Select engineer(s) or no engineer required
  • Select scientist(s) or no scientist required

Labels

  • Select component(s)
  • Select priority
  • Select requestor(s)

Projects and Milestone

  • Select Repository and/or Organization level Project(s) or add alert: NEED PROJECT ASSIGNMENT label
  • Select Milestone as the next official version or Future Versions

Define Related Issue(s)

Consider the impact to the other METplus components.

  • METplus, MET, METdataio, METviewer, METexpress, METcalcpy, METplotpy
    Recommend using the lessons learned in this process and checking for/fixing similar warnings in the PDF generation of the documentation for the MET, METplotpy, METcalcpy, METdataio, METviewer, and METexpress repositories.

Enhancement Checklist

See the METplus Workflow for details.

  • Complete the issue definition above, including the Time Estimate and Funding Source.
  • Fork this repository or create a branch of develop.
    Branch name: feature_<Issue Number>_<Description>
  • Complete the development and test your changes.
  • Add/update log messages for easier debugging.
  • Add/update unit tests.
  • Add/update documentation.
  • Add any new Python packages to the METplus Components Python Requirements table.
  • Push local changes to GitHub.
  • Submit a pull request to merge into develop.
    Pull request: feature <Issue Number> <Description>
  • Define the pull request metadata, as permissions allow.
    Select: Reviewer(s) and Development issues
    Select: Repository level development cycle Project for the next official release
    Select: Milestone as the next official version
  • Iterate until the reviewer(s) accept and merge your changes.
  • Delete your fork or branch.
  • Close this issue.
@JohnHalleyGotway JohnHalleyGotway added type: enhancement Improve something that it is currently doing component: documentation Documentation issue priority: medium Medium Priority alert: NEED ACCOUNT KEY Need to assign an account key to this issue labels Mar 16, 2023
@JohnHalleyGotway JohnHalleyGotway added this to the METplus-5.1.0 milestone Mar 16, 2023
@jprestop
Copy link
Collaborator

HI @JohnHalleyGotway and @georgemccabe. I was attempting to help @lisagoodrich get started on this task. You said:

Recommend starting by building the PDF documentation locally on a laptop and counting up the number of warning messages:

I wasn't sure if her laptop would have everything needed to build this, so instead, we used seneca to have her build. Unfortunately, her environment was using Python 3.8, but Python 3.10 was needed. We tried having her run:

conda activate /home/met_test/.conda/envs/metplus_dev.v5.1

but that did not work for her, so we requested she be added to the met_test group, as I did not get the same Python error she did. I received the following error:

Extension error:
Could not import extension sphinx_design (exception: No module named 'sphinx_design')

but was able to add sphinx-design to the metplus_dev.v5.1 environment as the met_test user, which resolved that problem. Then we ran the following:

cd docs
make clean latexpdf | tee make_docs_pdf.log
egrep -i "warning" make_docs_pdf.log | wc -l

as indicated, but make clean latexpdf | tee make_docs_pdf.log 👍

(metplus_dev.v5.1) jpresto@seneca:/d1/personal/jpresto/METplus/git/METplus-feature_2093_pdf_doc_warnings/docs$ make clean latexpdf | tee make_docs_pdf.log

simply produced a log of the build with no warnings. The only reference to warning in make_docs_pdf.log are from the branch name:

(metplus_dev.v5.1) jpresto@seneca:/d1/personal/jpresto/METplus/git/METplus-feature_2093_pdf_doc_warnings/docs$ grep -i warning make_docs_pdf.log
['/d1/personal/jpresto/METplus/git/METplus-feature_2093_pdf_doc_warnings', '/d1/personal/met_test/conda/envs/metplus_dev.v5.1/bin', '/home/met_test/.conda/envs/metplus_dev.v5.1/lib/python310.zip', '/home/met_test/.conda/envs/metplus_dev.v5.1/lib/python3.10', '/home/met_test/.conda/envs/metplus_dev.v5.1/lib/python3.10/lib-dynload', '/home/met_test/.conda/envs/metplus_dev.v5.1/lib/python3.10/site-packages', '/d1/personal/jpresto/METplus/git/METplus-feature_2093_pdf_doc_warnings/docs/_ext']
make[1]: Entering directory '/d1/personal/jpresto/METplus/git/METplus-feature_2093_pdf_doc_warnings/docs/_build/latex'
make[1]: Leaving directory '/d1/personal/jpresto/METplus/git/METplus-feature_2093_pdf_doc_warnings/docs/_build/latex'

This make_docs_pdf.log file is located at seneca:/d1/personal/jpresto/METplus/git/METplus-feature_2093_pdf_doc_warnings/docs in case you want to take a look.

I'm not sure why we are not seeing the 11,440 warning that you refer to above. It looks like we need some further guidance on getting started with finding these warnings.

@jprestop
Copy link
Collaborator

Hello again @JohnHalleyGotway and @georgemccabe. @lisagoodrich and I decided to go look in the build on RTD for warnings and it looks like we can see warnings there under the following command:

latexmk -r latexmkrc -pdf -f -dvi- -ps- -jobname=metplus -interaction=nonstopmode

I'm a little perplexed why we don't see these locally, but at least we have a starting point for now.

@jprestop
Copy link
Collaborator

@JohnHalleyGotway and @georgemccabe. @lisagoodrich and I tried to get started but are having a difficult time finding the warnings listed in appropriate files and lines in the files. We'd love to have a brief working session with you when you two get back to show us how to find these warnings in the file and troubleshoot a couple of them to help get Lisa started.

@georgemccabe
Copy link
Collaborator

ReadTheDocs uses Python 3.8, not 3.10:

version: 3.8

We had tried to update the version there, but had to revert back because it failed. ReadTheDocs may support 3.10 now, I'm not sure. Perhaps using Python 3.8 to build the docs locally would recreate the errors?

@jprestop
Copy link
Collaborator

jprestop commented Apr 5, 2023

@georgemccabe It looks like ReadTheDocs is using Python 3.10 in their .readthedocs.yaml file:
https://github.com/readthedocs/readthedocs.org/blob/main/.readthedocs.yml#L18

@georgemccabe
Copy link
Collaborator

@jprestop, that is good to know. I am fairly sure we couldn't use 3.10 when we were updating the repos to use that version. Do the warnings still exist on the RTD website logs when 3.10 is used? That would be a good test. It is confusing to me that you aren't seeing any warnings locally. Perhaps they are being suppressed due to the arguments used in the Makefile?

@georgemccabe
Copy link
Collaborator

georgemccabe commented Apr 5, 2023

When I run make clean latexpdf using metplus_dev.v5.1, I get warnings/errors and it fails to build. Do you get a successful build?

EDIT: When I run the command with the pipe to tee, I see warning/error in the terminal but not in the log file output. Maybe the stderr output is not being sent to the file?

@jprestop
Copy link
Collaborator

jprestop commented Apr 5, 2023

@georgemccabe, I will let @lisagoodrich follow up on

Do you get a successful build?

@lisagoodrich
Copy link
Contributor

@georgemccabe & @jprestop I am past my ability level. I do not get a successful build. Below is the output:

(base) lisag@seneca:~/working_directory/git/METplus/docs$ make clean latexpdf
rm -rf _build/* ./generated/met_tool_wrapper ./generated/model_applications
[ -d _build ] || mkdir -p _build
Running Sphinx v6.1.3
['/d1/personal/lisag/git/METplus', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/bin', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/lib/python310.zip', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/lib/python3.10', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/lib/python3.10/lib-dynload', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/lib/python3.10/site-packages', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/lib/python3.10/site-packages/PyNIO', '/d1/personal/lisag/git/METplus/docs/_ext']

Extension error:
Could not import extension sphinx_gallery.gen_gallery (exception: No module named 'sphinx_gallery')
make: *** [Makefile:27: latexpdf] Error 1
(base) lisag@seneca:~/working_directory/git/METplus/docs$

base) lisag@seneca:/working_directory/git/METplus$ cd docs
(base) lisag@seneca:
/working_directory/git/METplus/docs$ make clean latexpdf | tee make_docs_pdf.log
rm -rf _build/* ./generated/met_tool_wrapper ./generated/model_applications
[ -d _build ] || mkdir -p _build
Running Sphinx v6.1.3

Extension error:
Could not import extension sphinx_gallery.gen_gallery (exception: No module named 'sphinx_gallery')
['/d1/personal/lisag/git/METplus', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/bin', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/lib/python310.zip', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/lib/python3.10', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/lib/python3.10/lib-dynload', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/lib/python3.10/site-packages', '/var/autofs/mnt/linux-amd64/debian/buster/local/anaconda3-20230129/lib/python3.10/site-packages/PyNIO', '/d1/personal/lisag/git/METplus/docs/_ext']
make: *** [Makefile:27: latexpdf] Error 1
(base) lisag@seneca:/working_directory/git/METplus/docs$ egrep -i "warning" make_docs_pdf.log | wc -l
0
(base) lisag@seneca:
/working_directory/git/METplus/docs$

@jprestop
Copy link
Collaborator

jprestop commented Apr 6, 2023

@lisagoodrich I think you may be using the wrong environment. I think you need to run the following in bash:

conda activate /home/met_test/.conda/envs/metplus_dev.v5.1

Please try that and rerun.

@lisagoodrich
Copy link
Contributor

@jprestop thanks for the reminder about the conda environment (I forgot ... again.)

I got a lot of output. But I'm still not getting a nice list of "warning"s that I can work on. Since this was 27 pages of output, I threw the screen output into a google doc for your reference.
https://docs.google.com/document/d/1otsfokk1sI2Rz4UCJpD4kuW6LqTwOledVTlIbv6M0JQ/edit?usp=sharing
What is the next step?

@georgemccabe
Copy link
Collaborator

I'm not sure how to get the PDF to build using make. You could try using the command in the ReadTheDocs logs that contains the warnings to recreate them locally. I would try to do that myself to help but I don't think I will be able to get to it before I leave on PTO.

@georgemccabe georgemccabe changed the title Investigate warnings in the PDF documentation generation process Documentation: Investigate warnings in the PDF documentation generation process Apr 10, 2023
@lisagoodrich
Copy link
Contributor

@JohnHalleyGotway, @georgemccabe & @jprestop this is beyond my troubleshooting abilities. Julie showed me how to see the errors in read the docs. We can see the warnings on a build of METplus (for example https://readthedocs.org/projects/metplus/builds/20071221/ clicking on "latexmk -r latexmkrc -pdf -f -dvi- -ps- -jobname=metplus -interaction=nonstopmode"), but it is not clear how to solve these warnings, despite looking up information online. There are no line numbers corresponding to the warnings.
I'm sorry but I'm going to need more guidance to move forward on this.

@jprestop
Copy link
Collaborator

jprestop commented Apr 13, 2023

Thank you @lisagoodrich. I agree with your comments. I am hoping that since @JohnHalleyGotway was able to work through some of these for MET, he will be able to have a brief working session to show us how to find the warnings in the proper place in the documentation. @lisagoodrich and I went through a few of the warnings trying to figure out where they were located so we could solve them, but had trouble. @JohnHalleyGotway when you have some time after the beta2 release, could you please provide some guidance?

@jprestop jprestop added reporting: DTC NCAR Base NCAR Base DTC Project reporting: DTC NOAA BASE NOAA Office of Atmospheric Research DTC Project labels Apr 13, 2023
@lisagoodrich
Copy link
Contributor

Hi @JohnHalleyGotway,
Now that the release is done, would you be able to have a brief working session to show @jprestop and I how to find the warnings in the proper place in the documentation? And then walk us through solving a couple of them.

@jprestop jprestop modified the milestones: METplus-5.1.0, METplus-6.0.0 Jul 18, 2023
@JohnHalleyGotway
Copy link
Collaborator Author

I tested this for the MET documentation as well and see 2947 warnings:

make latexpdf | tee make_latexpdf.log
egrep -i warning make_latexpdf.log  | wc -l
2947

1566: Package fancyhdr Warning: \headheight is too small (12.0pt):
48: Package hyperref Warning: Token not allowed in a PDF string (Unicode):
608: LaTeX Warning: Reference
608: LaTeX Warning: Hyper reference
117: Some combination of...

LaTeX Font Warning: Font shape `T1/zi4/m/it' undefined
LaTeX Font Warning: Some font shapes were not available, defaults substituted.
LaTeX Warning: Command \textellipsis invalid in math mode on input line 38743.
LaTeX Warning: Command \textellipsis invalid in math mode on input line 38746.
LaTeX Warning: Label(s) may have changed. Rerun to get cross-references right.
LaTeX Warning: There were undefined references.
Package amsmath Warning: Foreign command \over;
Package longtable Warning: Column widths have changed
Package longtable Warning: Table widths have changed. Rerun LaTeX.
Package rerunfilecheck Warning: File `users_guide.out' has changed.
Package textcomp Warning: Symbol \textasciigrave not provided by
Package textcomp Warning: Symbol \textopenbullet not provided by

@lisagoodrich
Copy link
Contributor

It seems it's easy to get a list of Warnings but not so easy to get a line number within the documentation to fix these. I'm wondering if we could reach out to @ksearight who created this with ease in the first place. He may have some tricks up his sleeve to get us up and running on this.
I've set up a meeting on Friday at 11am to discuss this. Hopefully we won't need the full hour. (Our 4 schedules are pretty full.)

@JohnHalleyGotway
Copy link
Collaborator Author

@lisagoodrich I did some googling about one of these warnings and suspect that there isn't anything we actually can or need to do to address it:

https://tex.stackexchange.com/questions/327285/what-does-this-warning-mean-fancyhdr-and-headheight

The steps for rendering these .rst files as PDF's can be seen in one of the RTD run logs:

python -m sphinx -T -E -b latex -d _build/doctrees -D language=en . $READTHEDOCS_OUTPUT/pdf
latexmk -r latexmkrc -pdf -f -dvi- -ps- -jobname=metplus -interaction=nonstopmode

I don't think there's actually anything that lives in these repos that'd have any impact on those warning messages.

Scrolling through those warning message, I'm mostly concerned about the ones that mention undefined references. For example:

LaTeX Warning: Hyper reference `Users_Guide/release-notes:upgrade-instructions'
 on page 7 undefined on input line 1298.

Perhaps all we should do for this issue is do a light manual review of the PDF document to give us confidence that none of the warning messages in the build log actually manifest as problems in the PDF? I'm getting the sense that latexmk spitting out lots and lots of warning messages is just what happens through RTD, and we probably don't need to worry about it.

@lisagoodrich
Copy link
Contributor

@JohnHalleyGotway, I looked into this specific case. If you search this page, https://github.com/dtcenter/METplus/blob/main_v5.1/docs/Users_Guide/release-notes.rst for "upgrade-instructions" it pulls up here. (I am viewing it in raw mode):

  • METplus Wrappers (latest <https://metplus.readthedocs.io/en/latest/Users_Guide/release-notes.html>, :ref:upgrade instructions <upgrade-instructions>, development <https://metplus.readthedocs.io/en/develop/Users_Guide/release-notes.html>)

A reference doesn't need <>. (But a web link would need them). I think this should just be:
:ref:upgrade instructions upgrade-instructions

Here's how it looks in ReadTheDocs.
https://metplus.readthedocs.io/en/latest/Users_Guide/release-notes.html#upgrade-instructions

Would you like me to make this one change now to see if this fixes the problem?

@jprestop
Copy link
Collaborator

@lisagoodrich

I think that this may not be the case:

A reference doesn't need <>. (But a web link would need them). I think this should just be:
:ref:upgrade instructions upgrade-instructions

Looking at the role :ref: in the Sphinx Guide, it says:

Labels that aren’t placed before a section title can still be referenced, but you must give the link an explicit title, using this syntax: :ref:Link title <label-name>.

Since this label is being referenced before the section title (which comes later on the same page) I think it needs to have the <label-name>.

So it is unclear why this warning is being thrown at all, because it looks accurate to me:

LaTeX Warning: Hyper reference `Users_Guide/release-notes:upgrade-instructions'
 on page 7 undefined on input line 1298.

Regarding @JohnHalleyGotway's comment:

Perhaps all we should do for this issue is do a light manual review of the PDF document to give us confidence that none of the warning messages in the build log actually manifest as problems in the PDF? I'm getting the sense that latexmk spitting out lots and lots of warning messages is just what happens through RTD, and we probably don't need to worry about it.

I think doing a light manual review of the PDF document is a good way to go. @lisagoodrich Would you be comfortable spending about 10 minutes before our scheduled meeting on Friday, taking a look at the PDF checking for

  • alignment issues
  • any obvious problems
  • spot check links for any broken links
  • and noting what you find here on this issue?

If @JohnHalleyGotway's research on the latexmk command seems to indicate that spitting out lots and lots of warning messages is just what happens through RTD, and your light manual review doesn't turn up much, maybe there won't be much to do here. You could also take a quick look and randomly pick other warnings to check (assuming you are able to find them since we don't have line numbers for the PDF to match up with the line number printed out in the warnings - great job finding this one!), but it's probably best to just start with a brief review of the PDF.

@lisagoodrich
Copy link
Contributor

@jprestop, I'm in over my head. Perhaps we can discuss this when we meet today?

I tried putting the above "upgrade-instructions" problem into the sphinx editor, http://seneca.rap.ucar.edu:5000/ (I copied the raw text into the browser. Use Firefox. It won't work in chrome.) I couldn't make any progress. In fact, it gives me a different result. The text is in italics. I'm out of ideas.

@jprestop
Copy link
Collaborator

@JohnHalleyGotway @georgemccabe Just wanted to update here. @lisagoodrich and I met this morning. @lisagoodrich is going to do a light manual review of the PDF as described above and will post noteworthy problems here for discussion at our meeting tomorrow.

@lisagoodrich
Copy link
Contributor

@jprestop Thank you very much for explaining to me that these problems were ONLY in the PDF documentation, not the rst file documentation. (I thought the problem originated in the rst files and continued into the PDF ooops.)

For reference, there are 2,357 pages in the PDF documentation.
I did a quick review of the first 308 pages, all the way through Chapter 6. (approximately 13% of the documentation.)
I clicked on 10 random links. They all worked.

The only obvious problems I could see were with the tables. See below.
Chapter 1 Overview
1.5 METplus Components Python Requirements - table runs off the page.

Chapter 5 Configuration
5.2.1.2 INPUT_BASE the code block is huge font and different colors but readable.

5.7.1.2 climo_cdf.cdf_bins table wording goes off the page.

Chapter 6 Python Wrappers
Table line runs over the next table line and are not readable.
${METPLUS_USER_WANTS_TO_TRACK_THICK500850}
${METPLUS_USER_WANTS_TO_TRACK_THICK200500}
${METPLUS_USER_WANTS_TO_TRACK_THICK200850}
${METPLUS_USER_WANTS_TO_TRACK_ZETA850}
${METPLUS_CLIMO_MEAN_DICT}
${METPLUS_CLIMO_STDEV_DICT}

What would you like me to do? Or should we wait to discuss this tomorrow?

@jprestop
Copy link
Collaborator

@JohnHalleyGotway, @georgemccabe, @lisagoodrich, and @jprestop met today to try to debug some of these errors. We specifically focused on the overflow of table entries. There seems to be some correlation between the log warnings (from rst to latex, not from latex to pdf) and table width, but when we get some of the warnings to go away (by modifying the width) the table appears the same in the output PDF. One option is to learn more about table formatting and how it is rendered in PDF format (via latex). For now, we're going to close this issue and may potentially reopen it in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alert: NEED ACCOUNT KEY Need to assign an account key to this issue component: documentation Documentation issue priority: medium Medium Priority reporting: DTC NCAR Base NCAR Base DTC Project reporting: DTC NOAA BASE NOAA Office of Atmospheric Research DTC Project type: enhancement Improve something that it is currently doing
Projects
Status: 🏁 Done
Development

No branches or pull requests

4 participants