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

[ADAM-1548] Generate reStructuredText from pandoc markdown. #1646

Merged
merged 1 commit into from Dec 6, 2017

Conversation

@fnothaft
Copy link
Member

@fnothaft fnothaft commented Jul 27, 2017

Resolves #1548. Generates rst documentation from the pandoc markdown. To display on readthedocs, this generated documentation must be checked in, so we add integration to jenkins-test that makes sure that the generated docs are not out of date and fails the build if the markdown changes but the docs don't. Additionally, this commit adds the conf.py needed by readthedocs.

Here's the PDF documentation that this generates: https://media.readthedocs.org/pdf/adam/latest/adam.pdf

For whatever reason, this doesn't generate the HTML docs properly. Anyone have thoughts?

@fnothaft fnothaft added this to the 0.23.0 milestone Jul 27, 2017
@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Jul 27, 2017

As an alternative, how would people feel about me using the auto-conversion to port the existing pandoc markdown to rst, and us just cutting over to rst? That would avoid the need for regenerating the rst docs every time we update the markdown.

@heuermh
Copy link
Member

@heuermh heuermh commented Jul 27, 2017

Sorry, I am missing something, why generate rst from markdown? I thought Sphinx could run on markdown (CommonMark, specifically)?

@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Jul 27, 2017

This is pretty half baked TBH; I think 5 min after opening the PR, I realized that generating rst from markdown was kinda silly.

CommonMark is supported, but it sounds more limited than rst. I don't really have strong opinions one way or the other; I don't terribly love Markdown, and I think I like rst slightly more. If we have API docs that will automatically push to readthedocs, having markdown which renders on github seems like less of a sell.

@heuermh
Copy link
Member

@heuermh heuermh commented Jul 27, 2017

"We support Sphinx docs written with reStructuredText and CommonMark."
http://docs.readthedocs.io/en/latest/

  1. We support Sphinx docs written with reStructuredText, and
  2. We support CommonMark.

or

  1. We support Sphinx docs written with reStructuredText, and
  2. We support Sphinx docs written with CommonMark

?

@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Jul 27, 2017

@heuermh readthedocs uses Sphinx to render the docs, Sphinx supports both rst and CommonMark.

@heuermh
Copy link
Member

@heuermh heuermh commented Jul 27, 2017

Thanks, I think I'm reading the right documentation now. I don't have strong feelings one way or another.

@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Jul 27, 2017

Here's what things look like if I fully carve up the docs:

rtd
branch

@heuermh
Copy link
Member

@heuermh heuermh commented Jul 27, 2017

Some observations:

  • Github renders .rst files fairly well
  • .rst link syntax is a little odd, or perhaps there is an issue with conversion, e.g. the link in the first line after this header is broken http://adam.readthedocs.io/en/latest/api/#transforming-genomicrdds-via-spark-sql
  • In-line code blocks are visually annoying in the rendered readthedocs output. I'd suggest we reduce usage of these blocks in our paragraphs. One way of doing this would be to replace code blocks for class name and method name references with links to scaladoc.
  • Syntax highlighting in code blocks is not as nice as with Github, is it possible to define our own colors? The biggest issue for me is that comment lines look the same as code lines. In fact, key words within comment lines are styled.

Would you suggest we replace the markdown docs with these generated rst docs at some point?

@coveralls
Copy link

@coveralls coveralls commented Jul 27, 2017

Coverage Status

Coverage remained the same at 83.961% when pulling 2e18366 on fnothaft:issues/1548-generate-rst into c8a2202 on bigdatagenomics:master.

@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Jul 27, 2017

Would you suggest we replace the markdown docs with these generated rst docs at some point?

Yeah, I think we should.

.rst link syntax is a little odd, or perhaps there is an issue with conversion, e.g. the link in the first line after this header is broken http://adam.readthedocs.io/en/latest/api/#transforming-genomicrdds-via-spark-sql

The conversion process stripped all the anchors we had added. I'll need to make a pass and fix that.

In-line code blocks are visually annoying in the rendered readthedocs output. I'd suggest we reduce usage of these blocks in our paragraphs. One way of doing this would be to replace code blocks for class name and method name references with links to scaladoc.

I agree. RTD aside, they're not that great in the original markdown; our PDF/HTML docs are kind of hard to read as of now.

Syntax highlighting in code blocks is not as nice as with Github, is it possible to define our own colors? The biggest issue for me is that comment lines look the same as code lines. In fact, key words within comment lines are styled.

I don't think it has the Scala style attached right now. We should be able to find a Sphinx formatting color scheme for it.

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Jul 27, 2017

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2300/

Build result: FAILURE

[...truncated 15 lines...] > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1646/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains cf2ca17 # timeout=10Checking out Revision cf2ca17 (origin/pr/1646/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f cf2ca17d3e13a5c6b69a7e76443bc342f03c3b4aFirst time build. Skipping changelog.Triggering ADAM-prb ? 2.3.0,2.10,1.6.1,centosTriggering ADAM-prb ? 2.3.0,2.10,2.1.0,centosTriggering ADAM-prb ? 2.6.0,2.10,2.1.0,centosTriggering ADAM-prb ? 2.6.0,2.11,1.6.1,centosTriggering ADAM-prb ? 2.6.0,2.11,2.1.0,centosTriggering ADAM-prb ? 2.3.0,2.11,2.1.0,centosTriggering ADAM-prb ? 2.6.0,2.10,1.6.1,centosTriggering ADAM-prb ? 2.3.0,2.11,1.6.1,centosADAM-prb ? 2.3.0,2.10,1.6.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.10,2.1.0,centos completed with result FAILUREADAM-prb ? 2.6.0,2.10,2.1.0,centos completed with result FAILUREADAM-prb ? 2.6.0,2.11,1.6.1,centos completed with result FAILUREADAM-prb ? 2.6.0,2.11,2.1.0,centos completed with result FAILUREADAM-prb ? 2.3.0,2.11,2.1.0,centos completed with result FAILUREADAM-prb ? 2.6.0,2.10,1.6.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.11,1.6.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@heuermh
Copy link
Member

@heuermh heuermh commented Jul 28, 2017

Would you suggest we replace the markdown docs with these generated rst docs at some point?

Yeah, I think we should.

What do you think about sequencing then? Should we merge all the doc pull requests first and then convert after the content changes are done?

In-line code blocks are visually annoying in the rendered readthedocs output. I'd suggest we reduce usage of these blocks in our paragraphs. One way of doing this would be to replace code blocks for class name and method name references with links to scaladoc.

I agree. RTD aside, they're not that great in the original markdown; our PDF/HTML docs are kind of hard to read as of now.

Remembered something that I was frustrated with before, in ScalaDoc output you can link to a class name (ADAMContext) but not to a field or method, because there aren't any anchors.

Would it help to create a short style guide? E.g.

  • Class names - link to scaladoc
  • Method names - leave plain text
  • Command line arguments - inline code block
    etc.

Syntax highlighting in code blocks is not as nice as with Github, is it possible to define our own colors? The biggest issue for me is that comment lines look the same as code lines. In fact, key words within comment lines are styled.

I don't think it has the Scala style attached right now. We should be able to find a Sphinx formatting color scheme for it.

There is a scala lexer listed here
http://pygments.org/docs/lexers/

Looks like styles are defined in Pygments
http://pygments.org/docs/styles/

@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Jul 29, 2017

What do you think about sequencing then? Should we merge all the doc pull requests first and then convert after the content changes are done?

Yeah, I think that's probably the easiest way. The conversion process isn't too hard.

Would it help to create a short style guide? E.g.

SGTM! Add to CONTRIBUTING.md? Also, https://github.com/databricks/scala-style-guide ? I don't 100% agree with their style guide, but it is a start.

@gunjanbaid
Copy link
Contributor

@gunjanbaid gunjanbaid commented Aug 7, 2017

Some suggestions on content that can be added to conf.py:

import sphinx_rtd_theme

extensions = ['sphinx.ext.mathjax',	            # render LaTeX
    'sphinx.ext.githubpages']			    # publish on gh-pages, if we want to do this

# for using .rst files
source_suffix = '.rst'

# setting up readthedocs theme
html_theme = "sphinx_rtd_theme"
html_theme_path = [sphinx_rtd_theme.get_html_theme_path()]
@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Aug 7, 2017

Thanks @gunjanbaid! I'm thinking we'll fully drop .md files after this PR, but otherwise that LGTM.

@gunjanbaid
Copy link
Contributor

@gunjanbaid gunjanbaid commented Aug 7, 2017

@fnothaft Sounds good, I removed the lines needed for .md support in the above snippet.

@heuermh
Copy link
Member

@heuermh heuermh commented Aug 22, 2017

Remembered something that I was frustrated with before, in ScalaDoc output you can link to a class name (ADAMContext) but not to a field or method, because there aren't any anchors.

As of version 2.2.0, the Spark scaladocs have anchor links in them, e.g.
https://spark.apache.org/docs/latest/api/scala/index.html#org.apache.spark.rdd.RDD@collect():Array[T]

You find the permalink URL by hovering over the right side of the method block.

@fnothaft fnothaft force-pushed the fnothaft:issues/1548-generate-rst branch from 2e18366 to a342fe5 Dec 5, 2017
@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Dec 5, 2017

Majorly overhauled. Rebased on #1653 and subsumes #1645. You can build the HTML docs locally by cd'ing to docs and make htmling. You'll need Sphinx installed.

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Dec 5, 2017

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2500/
Test PASSed.

@heuermh
Copy link
Member

@heuermh heuermh commented Dec 5, 2017

Easier said than done?

docs (HEAD detached at fnothaft/issues/1548-generate-rst)
$ conda install sphinx
Fetching package metadata .................
Solving package specifications: .

Package plan for installation in environment /Users/xxx/.miniconda3:

The following NEW packages will be INSTALLED:

    alabaster:                0.7.10-py36_1         conda-forge
    babel:                    2.5.1-py36_0          conda-forge
    docutils:                 0.14-py36_0           conda-forge
    imagesize:                0.7.1-py36_0          conda-forge
    jinja2:                   2.10-py36_0           conda-forge
    markupsafe:               1.0-py36_0            conda-forge
    pygments:                 2.2.0-py36_0          conda-forge
    pytz:                     2017.3-py_2           conda-forge
    snowballstemmer:          1.2.1-py36_0          conda-forge
    sphinx:                   1.6.5-py36_0          conda-forge
    sphinxcontrib-websupport: 1.0.1-py36_0          conda-forge
    typing:                   3.5.2.2-py36_0        bioconda   

The following packages will be SUPERSEDED by a higher-priority channel:

    conda:                    4.3.30-py36h173c244_0             --> 4.3.29-py36_0 conda-forge
    conda-env:                2.6.0-h36134e3_0                  --> 2.6.0-0       conda-forge

Proceed ([y]/n)? y

conda-env-2.6. 100% |#############################################################################################| Time: 0:00:00 208.14 kB/s
alabaster-0.7. 100% |#############################################################################################| Time: 0:00:00 568.75 kB/s
docutils-0.14- 100% |#############################################################################################| Time: 0:00:00   2.92 MB/s
imagesize-0.7. 100% |#############################################################################################| Time: 0:00:00   1.41 MB/s
markupsafe-1.0 100% |#############################################################################################| Time: 0:00:00   5.31 MB/s
pytz-2017.3-py 100% |#############################################################################################| Time: 0:00:00   3.24 MB/s
snowballstemme 100% |#############################################################################################| Time: 0:00:00   8.93 MB/s
sphinxcontrib- 100% |#############################################################################################| Time: 0:00:00   6.42 MB/s
typing-3.5.2.2 100% |#############################################################################################| Time: 0:00:00   6.72 MB/s
babel-2.5.1-py 100% |#############################################################################################| Time: 0:00:00   7.24 MB/s
jinja2-2.10-py 100% |#############################################################################################| Time: 0:00:00   8.57 MB/s
pygments-2.2.0 100% |#############################################################################################| Time: 0:00:00   8.20 MB/s
conda-4.3.29-p 100% |#############################################################################################| Time: 0:00:00   7.74 MB/s
sphinx-1.6.5-p 100% |#############################################################################################| Time: 0:00:00   7.73 MB/s

docs (HEAD detached at fnothaft/issues/1548-generate-rst)
$ make html
sphinx-build -b html -d _build/doctrees   . _build/html
Running Sphinx v1.6.5
making output directory...

Exception occurred:
  File "conf.py", line 27, in <module>
    import sphinxcontrib.fulltoc
ModuleNotFoundError: No module named 'sphinxcontrib.fulltoc'
The full traceback has been saved in /var/folders/3y/61r1w_cs4hbdr_34nrdrbhww0000gn/T/sphinx-err-z_n0w9ge.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
make: *** [html] Error 1

docs (HEAD detached at fnothaft/issues/1548-generate-rst)
$ conda install sphinxcontrib-fulltoc
Fetching package metadata .................

PackageNotFoundError: Packages missing in current channels:
            
  - sphinxcontrib-fulltoc

We have searched for the packages in the following channels:
            
  - https://conda.anaconda.org/bioconda/osx-64
  - https://conda.anaconda.org/bioconda/noarch
  - https://conda.anaconda.org/conda-forge/osx-64
  - https://conda.anaconda.org/conda-forge/noarch
  - https://repo.continuum.io/pkgs/main/osx-64
  - https://repo.continuum.io/pkgs/main/noarch
  - https://repo.continuum.io/pkgs/free/osx-64
  - https://repo.continuum.io/pkgs/free/noarch
  - https://repo.continuum.io/pkgs/r/osx-64
  - https://repo.continuum.io/pkgs/r/noarch
  - https://repo.continuum.io/pkgs/pro/osx-64
  - https://repo.continuum.io/pkgs/pro/noarch
  - https://conda.anaconda.org/r/osx-64
  - https://conda.anaconda.org/r/noarch

docs (HEAD detached at fnothaft/issues/1548-generate-rst)
$ pip install sphinxcontrib-fulltoc
Collecting sphinxcontrib-fulltoc
  Downloading sphinxcontrib-fulltoc-1.2.0.tar.gz
Building wheels for collected packages: sphinxcontrib-fulltoc
  Running setup.py bdist_wheel for sphinxcontrib-fulltoc ... done
  Stored in directory: /Users/xxx/Library/Caches/pip/wheels/0b/aa/d9/9f07020081c43cfcabf4f92d2d21d56328dedf62ec5c5936b3
Successfully built sphinxcontrib-fulltoc
Installing collected packages: sphinxcontrib-fulltoc
Successfully installed sphinxcontrib-fulltoc-1.2.0

docs (HEAD detached at fnothaft/issues/1548-generate-rst)
$ make html
sphinx-build -b html -d _build/doctrees   . _build/html
Running Sphinx v1.6.5

Exception occurred:
  File "conf.py", line 37, in <module>
    raise RuntimeError('A virtualenv must be active and Sphinx must be installed in it')
RuntimeError: A virtualenv must be active and Sphinx must be installed in it
The full traceback has been saved in /var/folders/3y/61r1w_cs4hbdr_34nrdrbhww0000gn/T/sphinx-err-50i6yyt5.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
make: *** [html] Error 1

Sorry, conda, virtualenv, pip, I can't keep it all straight.

@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Dec 5, 2017

Yeah, I did this all in a virtualenv to eliminate said problems.

@fnothaft fnothaft force-pushed the fnothaft:issues/1548-generate-rst branch from a342fe5 to 129e3d7 Dec 5, 2017
@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Dec 5, 2017

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2501/
Test PASSed.

@heuermh
Copy link
Member

@heuermh heuermh commented Dec 5, 2017

Wot?

(tmp) docs (HEAD detached at fnothaft/issues/1548-generate-rst)
$ make html
sphinx-build -b html -d _build/doctrees   . _build/html
Running Sphinx v1.6.5

Exception occurred:
  File "conf.py", line 40, in <module>
    import bdgenomics.workflows.version
ModuleNotFoundError: No module named 'bdgenomics'
The full traceback has been saved in /var/folders/3y/61r1w_cs4hbdr_34nrdrbhww0000gn/T/sphinx-err-v926ab8y.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
make: *** [html] Error 1
@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Dec 5, 2017

ah, that's my bad... will fix shortly.

@fnothaft fnothaft force-pushed the fnothaft:issues/1548-generate-rst branch from 129e3d7 to a200fc9 Dec 6, 2017
@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Dec 6, 2017

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2502/
Test PASSed.

@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Dec 6, 2017

Pinging this for review/merge.

@heuermh
Copy link
Member

@heuermh heuermh commented Dec 6, 2017

Got this to build locally using sphinx and it looks generally pretty good.

I've found some problems with links but I'm inclined to merge this, update the latest URL on readthedocs to point to master, and then create a follow-on to fixup the nits. What do you think?

E.g.

a known variation database (e.g., dbSNP). {#known-snps}
See candidate generation and realignment. {#known-indels}
While the `transformAlignments <#transformAlignments>`__ command
@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Dec 6, 2017

@heuermh that SGTM, especially because that'll let us start debugging any readthedocs issues.

@heuermh heuermh merged commit 163217b into bigdatagenomics:master Dec 6, 2017
2 checks passed
2 checks passed
Codacy/PR Quality Review Good work! A positive pull request.
Details
default Merged build finished.
Details
@heuermh
Copy link
Member

@heuermh heuermh commented Dec 6, 2017

Thank you, @fnothaft!

@heuermh
Copy link
Member

@heuermh heuermh commented Dec 6, 2017

@fnothaft can you update the readthedocs config? It looks like all that needs to be done is to remove the branch from latest in Advanced Settings, but perhaps there is more to it than that.

This was referenced Dec 13, 2017
@heuermh heuermh added this to Completed in Release 0.23.0 Jan 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.