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

Docs (User + Design Guides) conversion to Sphinx #2910

Merged

Conversation

sadielbartholomew
Copy link
Collaborator

@sadielbartholomew sadielbartholomew commented Dec 21, 2018

This PR

Addresses #2651 (by title & opening description, where subsequent comments extend the scope). Minimal (:eight_spoked_asterisk:) reformulation of the core documentation to be auto-generated with Sphinx, to replace the LaTeX-sourced Cylc User & Suite Design Guides:

  • Configure the Sphinx documentation builder.
  • Convert all CUG & SDG content from TeX to reStructuredText format, as required by Sphinx.
  • Remove all now-defunct files under doc/, notably excluding the Cylc "index" homepage, which after this PR still remains & links across to the Sphinx docs index page.
  • Customise the 'classic' Sphinx theme to a clean, white take utilising the Cylc logo colours, which I really like & hope to persuade us to choose as the overall style 😉.
  • Modify cylc documentation accordingly & set-up a new admin command, cylc make-docs, to build the docs, which for the moment at least simply wraps the Sphinx HTML build command.

Review process

To build & view (from the new index page):

  1. Check out this branch.
  2. Run cylc make-docs to build.
  3. Run cylc documentation to access the HTML index page & navigate around e.g. via the sidebar to view.

For review, I suggest at least one reviewer needs to:

  • Check the two docs commands (still) function, ensuring the build runs without any errors or warnings.
  • Skim-read the entire Sphinx documentation in cross-reference with the current CUG & SDG to ensure no content has been omitted.
  • Go through all content & eyeball it to check the formatting is okay.

Note:

  • ❗ (as-is) requires no higher than Sphinx version 1.2.x, which is that on the MO RHEL6 environment, so downgrade your python-sphinx package if necessary. With a later version, you will get errors &/or warnings upon building. We should agree the version to converge upon going forward, then I can make amendments accordingly.
    version range >= 1.5.3, <= 1.7.9 agreed
  • the number of files changed is rather intimidating, but is largely due to the movement of the graphics files (find . -type f | wc -l = 158) & the fact that the .tex to .rst file extension changes are not registered as a split-up & rename. Only ~40 files essentially have been amended, mostly corresponding to partitioning the CUG into files for each section.

[Initial follow-on work listing moved into #2933.]

@sadielbartholomew sadielbartholomew added the doc Documentation label Dec 21, 2018
@sadielbartholomew sadielbartholomew added this to the next-release milestone Dec 21, 2018
@sadielbartholomew sadielbartholomew self-assigned this Dec 21, 2018
@sadielbartholomew sadielbartholomew changed the title Documentation (User + Design Guides) conversion to Sphinx Docs (User + Design Guides) conversion to Sphinx Dec 21, 2018
@sadielbartholomew sadielbartholomew force-pushed the docs-conversion-to-sphinx branch 4 times, most recently from ac10165 to 4732a6d Compare December 23, 2018 21:55
@matthewrmshin matthewrmshin modified the milestones: next-release, soon Jan 3, 2019
@sadielbartholomew sadielbartholomew requested review from oliver-sanders and removed request for matthewrmshin January 3, 2019 12:46
@sadielbartholomew
Copy link
Collaborator Author

Assigning @oliver-sanders given his in-depth knowledge of Sphinx & documentation overhaul. @matthewrmshin: I have unassigned you as the one of the formal reviewing pair, but of course feel free to provide feedback or review otherwise.

Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

Docs build OK under 1.7.9 but with some warnings which will get fixed when the theme is hardcoded. The HTML is pretty smooth, successfully de-LaTeXed!


- `mock <https://mock.readthedocs.io>`_

The User Guide is generated from LaTeX source files by running
Copy link
Member

Choose a reason for hiding this comment

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

This section might need updating :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah yes, well spotted. I'll just need to change is to was, right!? 😁

- And any include-files used in it (see below; may be
kept in sub-directories).

- **A ``bin/`` sub-directory** (optional)
Copy link
Member

Choose a reason for hiding this comment

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

Annoyingly RST can't handle a literal inside bold text, other markup languages can but RST is somewhat deficient here.

This will find other examples (along with some false positives):

$ git grep -E '\*[^\*]+`[^\*]+\*'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, I'll go through & amend those instances.

.. code-block:: cylc

# include the file $CYLC_SUITE_DEF_PATH/inc/foo.rc:
%include inc/foo.rc
Copy link
Member

Choose a reason for hiding this comment

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

This code-block gets rendered in red as an error, this is my fault (sorry) I didn't add the include file syntax to the cylc_lang lexer. These errors can be ignored in this PR.

[scheduling]
[[dependencies]]
[[[T00, T12]]]
graph = "my-foo<other.suite::foo> => bar"
Copy link
Member

Choose a reason for hiding this comment

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

Inter-suite triggers render in red as errors, my fault again, the cylc_lang lexer doesn't like these (probably the colon character). Again these errors can be ignored in this PR.

I've updated the table in #2752 accordingly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No problem, thanks for investigating. I did notice there were a few syntax highlighting abnormalities in the built docs & intended to add that into one of the lists for future work (& seem to have neglected to). But #2752 is a more appropriate home.

Copy link
Member

Choose a reason for hiding this comment

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

I've now fixed these issues in metomi/rose#2274, we can copy across the updated cylc_lang.py once it's merged (in a future PR if this gets merged first).

warm start) the cycle point at which it starts can be given on the command
line or hardwired into the suite.rc file:

.. code-block:: cylc
Copy link
Member

Choose a reason for hiding this comment

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

This code block renders in red as an error but this time it's actually a genuine error!

-.. code-block:: cylc
+.. code-block:: bash

There are a few other code blocks showing up in red (most are probably my fault, I'll fix the syntax file) some are genuine. Here is a list of the effected files:

$ grep -l --color=none -r '<span class="err"' | sed 's/\.html/\.rst/' | sed 's/^/doc\/src\//'
doc/src/install/html/built-sphinx/appendices/cylc-6-migration-ref.rst
doc/src/install/html/built-sphinx/appendices/suiterc-config-ref.rst
doc/src/install/html/built-sphinx/running-suites.rst
doc/src/install/html/built-sphinx/suite-config.rst
doc/src/install/html/built-sphinx/suite-design-guide/general-principles.rst
doc/src/install/html/built-sphinx/suite-design-guide/portable-suites.rst
doc/src/install/html/built-sphinx/suite-design-guide/style-guide.rst
doc/src/built-sphinx/appendices/cylc-6-migration-ref.rst
doc/src/built-sphinx/appendices/suiterc-config-ref.rst
doc/src/built-sphinx/running-suites.rst
doc/src/built-sphinx/suite-config.rst
doc/src/built-sphinx/suite-design-guide/general-principles.rst
doc/src/built-sphinx/suite-design-guide/portable-suites.rst
doc/src/built-sphinx/suite-design-guide/style-guide.rst

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice, thanks for extracting that as a list for me. I'll get those errors ironed out.

@@ -0,0 +1,485 @@
#!/usr/bin/env python2
Copy link
Member

Choose a reason for hiding this comment

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

This file can go as you have implemented another script to do the job.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, good spot; I forgot to remove this once I realised it was out of scope to adapt it to work for Cylc commands for this initial PR.

@@ -20,20 +20,16 @@

Copy link
Member

Choose a reason for hiding this comment

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

I think we can ditch this script as well, Sphinx can take care of handling multiple versions of the build docs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

[Addressing in latest comment; see 🔆.]

licensing


.. insert vertical whitespace else sidebar menu overhangs short page (ugly)
Copy link
Member

Choose a reason for hiding this comment

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

This hack might work for the HTML version (provided that a particular theme is used), however, if we change the theme or build to a different format (e.g. PDF, slides)...

We could probably fix this in the chosen theme either in the Sphinx repository or with a custom CSS file (which would only affect that theme).

@@ -0,0 +1,124 @@
#
Copy link
Member

Choose a reason for hiding this comment

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

This extension hasn't been updated since 2012 which makes it a little high risk. If we are lucky this functionality might be provided in a more contemporary version of Sphinx. The Cylc tutorial requires 1.5.3 so we will have to up this anyway.

If not it looks like someone has put it up on pypi which would at least save us bundling the script: https://pypi.org/project/sphinx_numfig/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

[Addressing in latest comment; see 🔆.]

@sadielbartholomew
Copy link
Collaborator Author


To register, & follow-up on, new points discussed with @oliver-sanders in person last week, including those covered in his previous feedback set (as tagged ':high_brightness:'):


  1. Overall, the key decision to make is on the range of Sphinx versions to permit as documented dependencies (& for the minimum, acknowledged in the conf.py under the needs_sphinx key):

    We should agree the version to converge upon going forward, then I can make amendments accordingly.

    Then we can adapt the set-up accordingly. Major constraints:

    • The plan is to eventually transfer the 'Cylc Tutorial' of the Rose docs to the Cylc docs, but the slides built from it form the resources used to give our internal Cylc training. Therefore we need to support the hieroglyph slide builder. Unfortunately it has a bug which was found in September but is yet to be fixed (see Fix documentation build error metomi/rose#2236) creating the restriction <= 1.7.9 for now at least. Also:

      The Cylc tutorial requires 1.5.3

    • (:high_brightness:)The earliest versions of Sphinx only have built-in support for figure reference by their caption i.e. if you wanted within a paragraph to allude & link to a figure you could only (without hacking each instance) achieve ... as in Figure <entire figure caption> ..., which naturally ruins the flow of the text. We would like LaTeX-like numbered figure references i.e. ... as in Figure <figure (auto-generated) number> .... For v. 1.3+, Sphinx supports a numref role which enables this, but otherwise one needs to use an external extension, as suggested here to avoid writing a custom script. I added the script, assuming v. 1.2 as a starting point, but will now remove it, use & hardcode a higher version, & apply the numref role in all cases, since we need to use a later version regardless (see first point).

    Once we have decided a suitable version range, we can adapt to it & check for full coverage. I also need to update cylc check-software, adding in Sphinx & removing the LaTeX User Guide checks. However, there appear to be inconsistencies by version for version-return methods for Sphinx. E.g. for my environment:

     python -c "import sphinx; print sphinx.__version__"

    gives an ImportError, though sphinx-build --version returns Sphinx (sphinx-build) 1.2.2.
    Furthermore rose check-software which essentially uses the former fails to pick up on my Sphinx package. For early versions, sphinx or variants therefof do not seem to be importable in Python. So we need to account for this (& adapt rose check-software).

  2. 🔆 The main drive of this PR is to get a basic 'no-frills' conversion Sphinx documentation merged as a baseline for follow-on work. I know the vertical whitespace & make-index.sh script are not ideal solutions & applied them simply to get the built docs to a state they are functional & readable. I agree these aspects should be replaced via some reformulation, but I personally think we should do this in a subsequent PR.

@kinow
Copy link
Member

kinow commented Jan 8, 2019

Can't help much as I really know very little about Sphinx. But I must say I am impressed by the amount of work done so quickly. Looking forward to being able to build and browse the documentation 👏 👏 . I enjoyed refreshing my memory on LaTeX... but Sphinx sounds a lot easy/simpler.

Could it be that sphinx gives that error only for Python 2.6.? For me

 python -c "import sphinx; print sphinx.__version__"

Gives:

  • 2.7
$  python -c "import sphinx; print sphinx.__version__"
1.8.3
  • 3.7
$ python -c "import sphinx; print(sphinx.__version__)"
1.7.9

Cheers
Bruno

@sadielbartholomew
Copy link
Collaborator Author

sadielbartholomew commented Jan 8, 2019

Ah, thank you @kinow you've solved it (import sphinx works for me with 2.7 too)! That will simplify things a lot :) Pesky 2.6 strikes again.

@oliver-sanders
Copy link
Member

oliver-sanders commented Jan 8, 2019

However, there appear to be inconsistencies by version for version-return methods for Sphinx.

The import sphinx thing is probably just to do with the way Sphinx has been installed, you may need to fiddle your PYTHONPATH to import the module directly without the sphinx-build wrapper. For "normal" installations this shouldn't be an issue.

To register, & follow-up on, new points

  1. Unless there is any reason not to, just use the same Sphinx version constraints as the Rose documentation (>=1.5.3, <1.8.0). We will be migrating the Cylc Tutorial as well as some the extensions (e.g. the animated workflow diagrams).

    Due to major changes to the Sphinx internals it's not a good idea to try supporting older versions (writing compatible extensions can be hell).

    If there are any modern Sphinx features that would make a big difference don't be afraid to up the minimum version from 1.5.3.

  2. Now we have Sphinx I don't think we need an installation/index.html so we can just delete the old make-index.html. If you want we can do this in the next PR.

@sadielbartholomew
Copy link
Collaborator Author

Now we have Sphinx I don't think we need an installation/index.html so we can just delete the old make-index.html

I originally thought this created a 'homepage' (as in e.g. https://cylc.github.io/cylc/documentation.html) which previously linked to the User & Suite Design Guides & I thought as a stepping-stone should change to linking straight to the Sphinx HTML docs index page. But on closer inspection this is not the case, so I will get rid of it. I also will work out where cylc/documentation.html is being served from & make sure it still gets generated (& links to the HTML Sphinx index as desired), at least for now.

@hjoliver
Copy link
Member

Nice work @sadielbartholomew (I had thought we'd have to wait much longer for this!).

@hjoliver
Copy link
Member

hjoliver commented Jan 10, 2019

Now we have Sphinx I don't think we need an installation/index.html so we can just delete the old make-index.html

I assume you mean src/make-index.sh and install/index.html? The original intention was to provide an "installable" directory tree of locally built (from LaTeX) PDF and HTML docs along with an index to them:
docs

... but yeah, I suppose we won't be needing that anymore, so go ahead and delete.

@hjoliver
Copy link
Member

I also will work out where cylc/documentation.html is being served from & make sure it still gets generated (& links to the HTML Sphinx index as desired), at least for now.

I assume you mean the main cylc web site? That is served from the gh-pages branch of the repository. When I make a new release, I remake the docs (PDF and HTML) then copy them out of the repo; then check out gh-pages; then copy the new docs in; then git push gh-pages to GitHub, which automatically re-builds the web site. Obviously this is a bit painful. Originally a separate gh-pages branch had to be used, as I recall. Maybe we can do it better now, with the new docs.(?).

doc/src/conf.py Outdated Show resolved Hide resolved
@hjoliver
Copy link
Member

hjoliver commented Jan 10, 2019

The generated HTML here is a massive improvement 🎆 🍾 🎈

@sadielbartholomew
Copy link
Collaborator Author

sadielbartholomew commented Jan 10, 2019

@hjoliver thanks for the explanation in #2910 (comment), I was really quite puzzled as to where that page was emerging from! I thought it must be something to do with install/index.html, despite the code to generate that not corresponding much, through text insertion via templating or similar.

Maybe we can do it better now, with the new docs.(?).

I am sure we can, but would it be okay to add this to the list of possible follow-on work, instead of including it in this PR? I am keen to get this in with minimal complications, especially since any new changes to the docs on master need to be incorporated manually (rather than via git merge) to this PR, so they are in RST not LaTeX form.

Given the procedure you outlined, I assume you can just link to the Sphinx docs instead of the User Guide PDF etc., as before, when you push the gh-pages?

This all relates to the point from my original comment (as below). What additional documents do we want to include or link to in what will become the main index page of the Cylc docs?

Further inclusions & role of the 'homepage': should the Sphinx docs eventually become the main docs homepage, with the extra content from this Cylc 'homepage' i.e. under 'Presentations' & all subsequent sections, be incorporated?

@oliver-sanders
Copy link
Member

oliver-sanders commented Jan 10, 2019

Maybe we can do it better now, with the new docs.(?).

I am sure we can, but would it be okay to add this to the list of possible follow-on work

@hjoliver I think we are going to have a small run of PRs for the docs where we bring across features from the Rose docs, get PDF output, streamline the build -> deploy process, etc.

@matthewrmshin matthewrmshin added this to the soon milestone Jan 16, 2019
Configure Sphinx documentation builder
Copy all CUG & SDG text & split into .rst files w/ better sectioning
Convert all text content from TeX to RST
Create basic docs-building wrapper command 'make-docs'
Update 'cylc make-docs' & 'cylc documentation' accordingly
Add numfig extension to get numerical figure refs in Sphinx 1.2.x
Rebase branch to updated master to include all recent changes
Amend Cylc homepage 'index' page
Amend .gitignore to include all generated docs content
Amend documentation/00-make.t to account for new make method
Amend relevant tests to account for changed [documentation][files]
Adapt for clean build with sphinx version range >= 1.5.3, <= 1.7.9:
  * apply available built-in to remove external extension numfig
  * "classic" theme now hardcoded as required for these versions
Fix literals inside bold & (amendable) cylc_lang lexer error cases
Update 'check-software' command & installation instruction text
Remove redundant files (auto_cli_doc.py, make-index.sh)
@sadielbartholomew
Copy link
Collaborator Author

sadielbartholomew commented Jan 18, 2019

New commit # 1

#2924 is now incorporated, with the 'conflicts' it created (actually just the file deleted early on) resolved.

...and the same for newly-merged #2930.

New commit # 2

I did spot one (hopefully the last) component requiring a change to be made, namely the specified Cylc version; during development I manually set references to this to 7.8.0 & forgot to update them (not just to .1 but to automatically extract the system current version.)

The system/docs version registered by Sphinx is a setting in the conf.py. Ideally, & as we will be able to do when Cylc becomes a package in #2834, we could just fetch cylc.__version__ directly within that file, but at the moment that cannot be done, I believe, & I can't see another simple way to extract the version via Python.

The approach I took was to extract it via cylc --version in the building bash script, bin/cylc-make-docs, & pass it through as a setting=value to be inserted into the configuration pre-build. Then for the textual reference at the top of the index page, where I can't do the same, I used a substitution. This seems good enough for now (i.e. until Cylc 8)?

@sadielbartholomew
Copy link
Collaborator Author

(I think we can ignore the codecov failure, given the context of this being documentation & that we want to get this in ASAP.)

@sadielbartholomew sadielbartholomew modified the milestones: soon, next-release Jan 19, 2019
doc/src/conf.py Outdated Show resolved Hide resolved
doc/src/installation.rst Outdated Show resolved Hide resolved
doc/src/installation.rst Outdated Show resolved Hide resolved
@oliver-sanders
Copy link
Member

I think we can ignore the codecov failure

We can ignore the codecov failure as you have only changed two lines!

@oliver-sanders oliver-sanders merged commit 457c89b into cylc:master Jan 22, 2019
@hjoliver
Copy link
Member

Merged! 💥

@sadielbartholomew
Copy link
Collaborator Author

I'll get an Issue opened to collate further work we could do for enhancements, when other priorities are cleared. This will largely be from movement of text from my initial comment, but I will migrate uncompleted items contained in #2651 to this new Issue, so it's distinct end goal according to its strict title, i.e. the documentation conversion, can be marked as closed.

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

Successfully merging this pull request may close these issues.

5 participants