Added shim that allows us to use m2r components, but recommonmark mainly

[dotsdl]

Addresses #598

This PR fixes the doc build, which uses Sphinx 3.x. m2r, which we used for markdown handling, appears unmaintained and fails when building the docs under this version of Sphinx.

I used this approach to use the components we need from m2r (.. mdinclude:: directive), but recommonmark for everything else.

I would like a second set of eyes on the built docs to ensure everything is being generated as expected.

Also made some changes to makefile in line with latest sphinx (3.x) config generation. This may not necessarily be desirable; review requested.

• Tag issue being addressed
• Add tests
• Update docstrings/documentation, if applicable
• Update changelog

[dotsdl]

[Toolkit] Docs are broken

[mattwthompson]

This was not pulled down for me with conda env create --name openff-docs --file environment.yml

[mattwthompson]

This builds locally for me and looks good (after grabbing recommonmark). I looked over a handful of pages and nothing seemed out of the ordinary - anything in particular to check?

[j-wags]

Looking at this now -- I've tagged this branch for builds on RTD, so we should be able to see status here: https://readthedocs.org/projects/open-forcefield-toolkit/builds/

[j-wags]

Thanks for taking a swing at this. Right now, the API links in the release notes aren't connecting up -- compare clicking on the ParameterHandler.attribute_is_cosmetic method name in the new docs to that in the 0.6.0 release docs

I wonder if it's something to do with the m2r_parse_relative_links setting here

If this ends up being hard, I'd be happy to review and approve without the links working (since something is better than nothing). But I direct a lot of user traffic to our release notes, so if we can keep them that'd be ideal.

[dotsdl]

Just tried switching m2r_parse_relative_links to True as @j-wags suggested in here. Appears to produce the same effect. 😢

[dotsdl]

Tried this approach locally just now. Doesn't require recommonmark, but also doesn't fix the issue with broken API links. Going to push this so our dependencies remain the same, though it doesn't advance our approach much further.

[j-wags]

I wonder if it'd be worthwhile to take another go at pinning to an old version of sphinx. If we can force it down to 2.4.4, then all of our problems are solved (for a while).

[dotsdl]

I wonder if it'd be worthwhile to take another go at pinning to an old version of sphinx. If we can force it down to 2.4.4, then all of our problems are solved (for a while).

Just tried to set sphinx == 2.4 in conda docs/environment.yml, but it looks like the build installs sphinx after running this, overriding that specification. Any ideas on other knobs we can turn here?

[jaimergp]

Maybe this helps: readthedocs/readthedocs.org#5810

[j-wags]

We might be able to force a "conda" style build instead of a "sphinx" style one, and then just manually install the dependencies, including sphinx=2, as conda packages.

[dotsdl]

We might be able to force a "conda" style build instead of a "sphinx" style one, and then just manually install the dependencies, including sphinx=2, as conda packages.

We are already doing a conda style build. I have added an explicit sphinx pin to our docs/environment.yml, but this gets overwritten by the build, which runs the following right after installing our environment:

> conda install --yes --quiet --name doc-fix mock pillow sphinx sphinx_rtd_theme
Collecting package metadata (current_repodata.json): ...working... done
Solving environment: ...working... done

## Package Plan ##

  environment location: /home/docs/checkouts/readthedocs.org/user_builds/open-forcefield-toolkit/conda/doc-fix

  added / updated specs:
    - mock
    - pillow
    - sphinx
    - sphinx_rtd_theme


The following packages will be downloaded:

    package                    |            build
    ---------------------------|-----------------
    ca-certificates-2020.1.1   |                0         125 KB
    certifi-2020.4.5.1         |           py37_0         155 KB
    mock-4.0.2                 |             py_0          32 KB
    openssl-1.1.1g             |       h7b6447c_0         2.5 MB
    pillow-7.1.2               |   py37hb39fc2d_0         603 KB
    sphinx-3.0.3               |             py_0         1.1 MB
    sphinx_rtd_theme-0.4.3     |             py_0         4.4 MB
    ------------------------------------------------------------
                                           Total:         9.0 MB

The following NEW packages will be INSTALLED:

  mock               pkgs/main/noarch::mock-4.0.2-py_0
  sphinx_rtd_theme   pkgs/main/noarch::sphinx_rtd_theme-0.4.3-py_0

The following packages will be UPDATED:

  sphinx                     conda-forge::sphinx-2.4.0-py_0 --> pkgs/main::sphinx-3.0.3-py_0

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

  ca-certificates    conda-forge::ca-certificates-2020.4.5~ --> pkgs/main::ca-certificates-2020.1.1-0
  certifi            conda-forge::certifi-2020.4.5.1-py37h~ --> pkgs/main::certifi-2020.4.5.1-py37_0
  openssl            conda-forge::openssl-1.1.1g-h516909a_0 --> pkgs/main::openssl-1.1.1g-h7b6447c_0
  pillow             conda-forge::pillow-7.1.2-py37h718be6~ --> pkgs/main::pillow-7.1.2-py37hb39fc2d_0


Preparing transaction: ...working... done
Verifying transaction: ...working... done
Executing transaction: ...working... done 

I don't see any additional options in the RTD conda docs for overriding this behavior.

[dotsdl]

Besides the .. mdinclude directive, is there anything else out of m2r that we actually need? If not, we could move the spec into the doc tree and try sticking with recommonmark. This may sidestep the problem entirely.

[j-wags]

Besides the .. mdinclude directive, is there anything else out of m2r that we actually need?

I don't think so.

If not, we could move the spec into the doc tree and try sticking with recommonmark.

We screwed up here, and have a bunch of direct URL links to the latest branch spec docs that this might break. These links are spread all over the place, and I don't know how we'd track them down.

But here's my thinking:

• We'll break the SMIRNOFF spec links by migrating to GH Pages eventually, so we'll need to either do clever redirects or change all the existing direct-URL links anyway. But it'll be best to do that just once.
• Once we're hosting docs on GH Actions/Pages, we can pin to sphinx 2.4.4 (or take our time finding a workaround for docs links)
• Keeping the API links in releasenotes is very nice, but not critical
• The 0.7.0 release is time-critical, and so it's probably fine if we just have any working solution to get the docs building here

So, I'd actually propose that we just merge any successfully-building solution that we can now, even if it breaks API links from release notes. Then we can revisit this later when there's less on our plate.

[dotsdl]

Cool, went with one of the workarounds in conf.py that appeared to work well enough, and left the other one behind as a comment for now until we have a more permanent solution. As it happens, it appears that API references for methods, but not for e.g. classes, are broken, so we haven't lost all of the internal linking functionality we'd like.

[codecov-commenter]

Codecov Report

Merging #602 into master will not change coverage.
The diff coverage is n/a.