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

Bring documentation source to master #954

Merged
merged 248 commits into from
Apr 7, 2020
Merged

Conversation

billsacks
Copy link
Member

Description of changes

  1. Bring documentation source to master: Pulls in the source from
    https://github.com/escomp/ctsm-docs. This is important so that
    documentation can remain in sync with changes in the model
    code. Images are stored here using git-lfs (Git Large File
    Storage). I also made some minor fixes to get the pdf build of the
    tech note working.

  2. Use a different documentation theme that supports a version dropdown
    menu, and add the code needed to support this versioning on the
    documentation web pages. At a high level, the way the versioned
    documentation works is to have separate subdirectories in the
    gh-pages branch of the ctsm-docs repository for each version of the
    documentation we want to support. There is then a bit of JavaScript
    code which uses a json file in the gh-pages branch to determine which
    versions exist and how these should be named in the dropdown
    menu. Most of these changes were borrowed from
    Documentation: provide a version dropdown menu and change theme ESMCI/cime#3439, which in turn borrowed from
    Provide a version drop-down menu in documentation CISM-wrapper#23, which in turn was a
    slight modification of an implementation provided by @mnlevy1981 for
    the MARBL documentation, which in turn borrowed from an
    implementation put together by Unidata (credit where credit is due).

    I am not aware of out-of-the-box support for a version pull-down in
    out-of-the-box sphinx themes (though the last time I looked was in
    Fall, 2018, so there may be something available now). However,
    support for a version dropdown exists in an open PR in the sphinx
    readthedocs theme repository: Add a version selector dropdown to the main menu readthedocs/sphinx_rtd_theme#438. I
    have pushed this branch to a new repository in ESMCI
    (https://github.com/ESMCI/sphinx_rtd_theme) to facilitate long-term
    maintenance of this branch in case it disappears from the official
    sphinx_rtd_theme repository. I have also cherry-picked a commit onto
    that branch, which is needed to fix search functionality in sphinx1.8
    (from Fix HTML search not working with Sphinx-1.8. readthedocs/sphinx_rtd_theme#672) (which is another reason for
    maintaining our own copy of this branch). The branch in this
    repository is now named version-dropdown-with-fixes (branching off of
    the version-dropdown branch in the sphinx_rtd_theme repository). In
    the long-term, I am a little concerned about using this theme that
    isn't showing any signs of being merged to the main branch of the
    readthedocs theme, but this has been working for us in other projects
    for the last 2 years, so I feel this is a reasonable approach in the
    short-medium term.

Specific notes

Contributors other than yourself, if any: none

CTSM Issues Fixed (include github issue #):

Are answers expected to change (and if so in what way)? No

Any User Interface Changes (namelist or namelist defaults changes)? No

Testing performed, if any: None

Based on git-lfs/git-lfs#2717

The purpose of this is two-fold:

(1) It keeps clones and fetches lighter-weight

(2) It keeps our LFS bandwidth down, so we're less likely to need to
upgrade our account (though that may be free for our educational
account).
Get rid of the images external. Instead, put the images directly in the
users guide / tech note source. These images will be managed by git LFS.
This is needed because we have configured this repository (via an
.lfsconfig file at the top level) to NOT automatically fetch any of the
large files when cloning / fetching.
I'm going to add a pdf image file, so this is needed for that purpose.
This is needed for the pdf build: latexpdf cannot read svg files.

I converted this using Inkscape (simply using "save as" and saving it as
a pdf).
Previously, we had been (sort of) supporting builds of just the User's
Guide or just the Tech Note via near-duplicated copies of the Makefile
and conf.py file. However, these didn't really seem to work right for an
html build (the files ended up in the wrong directory structure), and
this was going to be harder to support and maintain moving forward,
because of the duplication. Erik Kluzek and Keith Oleson have confirmed
that they don't actually need support for an html build of just part of
the documentation. I have made changes to the main Makefile to support
just building a pdf of the tech note (not including the User's Guide),
which IS needed.
This changes the documentation theme to use the readthedocs theme, with
some JavaScript that provides capabilities for a dropdown menu allowing
you to select between different versions.

This mimics the changes in ESMCI/cime#3439,
which in turn was based on ESCOMP/CISM-wrapper#23.
This comes from altMITgcm/MITgcm#12, which I
found in a link from
readthedocs/sphinx_rtd_theme#383.

This right-aligns the equation numbers, and only makes the link image
visible if your mouse is hovering above a given equation.
@billsacks billsacks added PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete test: none No tests required (e.g. tools/contrib) labels Mar 30, 2020
@billsacks billsacks requested a review from ekluzek March 30, 2020 23:05
@billsacks billsacks self-assigned this Mar 30, 2020
@billsacks
Copy link
Member Author

@ekluzek - as I mentioned by email, this would be a beast to review, and I don't think the PR itself really requires a detailed review since most of the changes are simply pulling in the documentation source verbatim from the ctsm-docs repo, and it's more useful to review the resulting documentation build (https://billsacks.github.io/ctsm-docs).

However, if you want to review the interesting pieces of the PR, I suggest focusing on these commits:

@billsacks
Copy link
Member Author

I have updated the wiki page on the documentation:

https://github.com/ESCOMP/CTSM/wiki/Directions-for-editing-CLM-documentation-on-github-and-sphinx

@ekluzek as far as I'm concerned this is ready to come to master. But please let me know if you'd like me to wait for you do any further review before I merge it.

Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the pergro jpg files should be removed. There are a few changes that I ask for.

# built documents.
#
# The short X.Y version.
version = u'master'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking

version = u'CTSM1'
release = u'CTSM master'

@@ -0,0 +1,5 @@
.. |cesmrelease| replace:: CESM2.0.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"CESM2.0.0" should be "CESM2.2 beta"

# General information about the project.
project = u'ctsm'
copyright = u'2020, UCAR'
author = u''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think author should probably stay as

"Erik Kluzek, Bill Sacks, Ben Andre, Alice Bertini"

since we are the ones that worked with the bulk of the content. Or it could set set to

"CTSM software team"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree with this. I removed the authorship information because I felt it did not give credit to the appropriate people. There are many people who should get credit, as carefully laid out in https://escomp.github.io/ctsm-docs/versions/master/html/tech_note/Introduction/CLM50_Tech_Note_Introduction.html for the tech note. Furthermore, I'm not sure whether this authorship information actually gets used anywhere (it may, but if it does, I didn't see how at a glance), so it didn't feel worth putting a lot of effort into coming up with an accurate authorship list (especially given that this would be hard to maintain accurately).

@billsacks
Copy link
Member Author

@ekluzek I have addressed your comments about the versions. As noted above, I disagree about the authorship. If you feel strongly about this, I'd request that we bring this to master as it is now (so our collaborators can start working on their documentation) and then have a broader discussion on this point - though I'd like to see a demonstration that it actually makes a difference in the built documentation before putting any more thought into it. I'm not sure what the pergro jpg files are that you're referring to. Note that I removed the entire old UsersGuide directory, which included two pergro image files. Are there others that you still see somewhere?

@billsacks
Copy link
Member Author

@ekluzek - also: once you approve this, I'll do a final rebuild of the documentation.

@ekluzek
Copy link
Collaborator

ekluzek commented Apr 7, 2020

OK this is good. You are right about author not actually showing up. Note, that there are three conf.py files: one global one, one for the tech note and one for the UG. I was thinking about the authorship of the UG and NOT the Tech Note, which obviously has a ton of authors. And the authorship of each is completely different.

But, you are also right that it appears that the author replacement string isn't actually used anywhere. So it doesn't actually matter.

@billsacks
Copy link
Member Author

@ekluzek thanks for your comment. There used to be three different conf.py files, but that's no longer the case: now there is just one. I removed the separate conf.py files for the tech note and user's guide for two reasons:

  1. It was going to be a pain to try to keep these three files in sync moving forward

  2. It didn't seem like these tech note and user's guide conf.py files were actually used... at least, not consistently. From what I could tell, if you built the full documentation, the top-level conf.py file was used for both the tech note and user's guide (so the lower-level conf.py files were being ignored). The tech note's conf.py file was being used for the pdf build of the tech note, but I have gotten things working so that that can use the top-level conf.py file, too, and give the same results as before.

@billsacks billsacks merged commit b0610ed into ESCOMP:master Apr 7, 2020
@billsacks billsacks deleted the add_doc_source branch April 7, 2020 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete test: none No tests required (e.g. tools/contrib)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move documentation source to master
9 participants