Replaces astropy-helper with sphinx-astropy #2494

yashkgp · 2018-03-01T18:43:52Z

Solves Issue #2489

Cadair · 2018-03-01T20:06:40Z

.rtd-environment.yml

@@ -61,4 +61,4 @@ dependencies:
 - glymur
 - pip:
    - sunpy-sphinx-theme
-    - git+https://github.com/sphinx-gallery/sphinx-gallery


sphinx-gallery and sphinx-astropy do two different things we are going to need both 😄

Cadair · 2018-03-01T20:07:24Z

docs/conf.py

@@ -29,14 +29,7 @@
 import datetime
 import sys

-ON_RTD = os.environ.get('READTHEDOCS') == 'True'


I think we are still going to need all of this RTD stuff.

Should we check if we do need this?

See comment below, we are now importing it from sphinx astropy

Cadair · 2018-03-01T20:07:32Z

docs/conf.py

-    os.environ['HOME'] = '/home/docs/'
-    os.environ['LANG'] = 'C'
-    os.environ['LC_ALL'] = 'C'
+import astropy


I don't think we need this.

Fixes typo Small requested changes requested changes small changes

nabobalis · 2018-03-02T08:47:05Z

I think down the road we can remove sphinx-gallery since its one of the dependencies for sphinx-astropy. But since it has't had a release for a while, we still need to use master.

Issue is now we have one warning in the doc build but having a quick glance, I can't find it.

Cadair · 2018-03-02T10:48:24Z

/home/travis/build/sunpy/sunpy/build/lib.linux-x86_64-3.6/sunpy/spectra/spectrum.py:docstring of sunpy.spectra.spectrum.Spectrum:34: WARNING: Inline strong start-string without end-string.

Cadair · 2018-03-02T10:54:41Z

docs/conf.py

@@ -174,14 +166,13 @@
 napoleon_google_docstring = False

 # -- Options for the edit_on_github extension ----------------------------------------
-extensions.remove('astropy_helpers.extern.numpydoc')


we still need to remove this, jut from astropy_sphinx instead. It's possible this is what's causing the doc warning.

But extensions in astropy_sphinx does not contain astropy_helpers.extern.numpydoc, will raise an error if removed . Also numpydocs should be removed from extensions, right ?

@Cadair ping

yeah in astropy_sphinx it looks like it's just numpydoc we need to pop (https://github.com/astropy/sphinx-astropy/blob/master/sphinx_astropy/conf/v1.py#L135)

Cadair · 2018-03-02T11:10:30Z

I have just had a look through the conf.py file, and while this is very much not your fault, it's a bit of a mess, and could do with a tidy. This seems like an opportune time to tidy it up where appropriate.

Firstly, a quick bit of background. Sphinx imports conf.py and then uses any variables that are defined in that file to configure itself, i.e. it looks for a variable named extensions in conf.py to know what extensions to load (it will not use any variables in the file it is not expecting to be there).

How we organise our conf.py file is that we import the one from astropy_helpers or now sphinx_astropy and then modify things as we need to. This means we re-define some variables outright, and append or delete things from the lists and dictionaries we have imported from the astropy config.

What would be nice to do is to look through the conf/v1.py file in sphinx_astropy and find places where we duplicate things in that file or re-define things to be the same etc. Obvious ones I can see are intersphinx_mapping and on_rtd.

Then also feel free to re-format our conf.py to be as succinct as possible. The main changes we make to the default config in astropy_sphinx are:

Use our HTML theme
remove h5py from intersphinx and add sqlalchemy, pandas and skimage.
use napoleon instead of numpy doc
extra config for sphinx-gallery.

Also worth looking at is the conf.py for the sunpy theme, as it looks like we are re-defining one or two variables from that as well.

Thanks!

yashkgp · 2018-03-02T14:24:40Z

Thanks for the help @Cadair ,will update the conf.py ASAP.

Cadair · 2018-03-06T11:52:17Z

docs/conf.py

@@ -174,6 +172,9 @@
 napoleon_google_docstring = False
 extensions += ['sphinx_astropy.ext.edit_on_github', 'sphinx.ext.doctest', 'sphinx.ext.githubpages']

+# Remove numpydoc


Can you put this directly above the extensions.append('sphinx.ext.napoleon') line?

Also the napoleon config seems to be duplicated? Can you remove the one not under the "swap to napoleon" comment?

Cadair

I think this is very close! Just a couple of little tweaks.

Cadair · 2018-03-06T15:59:49Z

CHANGELOG.rst

@@ -42,6 +42,8 @@ Bug Fixes
 - Updates MapCube to access the correct properties of the namedtuple SpatialPair [#2297]
 - Fixed TimeSeries test failures due to missing test files [#2273]
 - Refactored a GOES test to avoid a Py3.6 issue [#2276]
+- The documentation build now uses the Sphinx configuration from sphinx-astropy 
+  rather than from astropy-helpers.


this needs the pr number in square brackets at the end.

Cadair · 2018-03-06T16:02:00Z

docs/conf.py

 extensions.append('sphinx.ext.napoleon')

 # Disable having a separate return type row
 napoleon_use_rtype = False
 # Disable google style docstrings
 napoleon_google_docstring = False

-# -- Options for the edit_on_github extension ----------------------------------------


This comment should still be here.

This was moved to the wrong part of the file?

Cadair · 2018-03-06T16:03:44Z

requirements/docs.txt

@@ -7,3 +7,4 @@
 sphinx
 sunpy-sphinx-theme
 sphinx-gallery
+sphinx_astropy


the pip name is sphinx-astropy not sphinx_astropy.

(yes, yes it's a very confusing convention lol)

nabobalis · 2018-03-07T11:44:59Z

docs/conf.py

 ON_TRAVIS = os.environ.get('TRAVIS') == 'true'

-if ON_RTD:
+if on_rtd:


Should be ON_RTD

No, we now import this from sphinx_astropy and it's lower case.

OH FAIR ENOUGH I SHOULD HAVE CHECKED

nabobalis · 2018-03-07T11:45:40Z

docs/conf.py

@@ -200,7 +185,7 @@

 # -- Options for the Sphinx gallery -------------------------------------------

-if ON_RTD and os.environ.get('READTHEDOCS_PROJECT').lower() != 'sunpy':
+if on_rtd and os.environ.get('READTHEDOCS_PROJECT').lower() != 'sunpy':


nabobalis · 2018-03-07T11:45:53Z

sunpy/database/tests/test_database.py

@@ -902,7 +902,8 @@ def test_fetch(database, download_query, tmpdir):
    database.undo()
    assert len(database) == 0
    database.redo()
-    assert len(database) == 4
+    # Make this resilitent to vso changes while we chase this up with VSO 2018-03-07
+    assert len(database) in (2, 4)


THIS IS MAGIC, WHY DIDNT I DO THIS.

Cadair · 2018-03-07T12:53:06Z

Thanks @yashkgp this is awesome!

Cadair reviewed Mar 1, 2018

View reviewed changes

Replaces astropy-helpers with sphinx-astropy

49ebb41

Fixes typo Small requested changes requested changes small changes

yashkgp force-pushed the issue2489 branch from 17e3ba7 to 49ebb41 Compare March 1, 2018 23:20

Cadair reviewed Mar 2, 2018

View reviewed changes

yashkgp added 2 commits March 6, 2018 02:48

Cleaned conf.py

23f0f96

Merge branch 'master' of https://github.com/sunpy/sunpy into issue2489

b84f81e

Cadair reviewed Mar 6, 2018

View reviewed changes

minor changes

f3adc54

yashkgp closed this Mar 6, 2018

yashkgp reopened this Mar 6, 2018

Cadair reviewed Mar 6, 2018

View reviewed changes

yashkgp and others added 3 commits March 6, 2018 23:11

Requested Changes

1702c00

Attempt to fix CI builds

98d0ce2

Merge branch 'master' of github.com:sunpy/sunpy into pr/2494

91ea8c6

Cadair approved these changes Mar 7, 2018

View reviewed changes

Cadair requested a review from nabobalis March 7, 2018 11:39

nabobalis reviewed Mar 7, 2018

View reviewed changes

nabobalis approved these changes Mar 7, 2018

View reviewed changes

nabobalis merged commit 069b8ab into sunpy:master Mar 7, 2018

nabobalis mentioned this pull request Mar 7, 2018

Replace astropy-helpers with sphinx-astropy #2489

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replaces astropy-helper with sphinx-astropy #2494

Replaces astropy-helper with sphinx-astropy #2494

yashkgp commented Mar 1, 2018

Cadair Mar 1, 2018

Cadair Mar 1, 2018

nabobalis Mar 7, 2018

Cadair Mar 7, 2018

Cadair Mar 1, 2018

nabobalis commented Mar 2, 2018 •

edited

Cadair commented Mar 2, 2018

Cadair Mar 2, 2018

yashkgp Mar 2, 2018 •

edited

yashkgp Mar 5, 2018

Cadair Mar 5, 2018

Cadair commented Mar 2, 2018

yashkgp commented Mar 2, 2018

Cadair Mar 6, 2018

Cadair left a comment

Cadair Mar 6, 2018

Cadair Mar 6, 2018

yashkgp Mar 6, 2018

nabobalis Mar 7, 2018

Cadair Mar 6, 2018

nabobalis Mar 7, 2018

Cadair Mar 7, 2018

nabobalis Mar 7, 2018

nabobalis Mar 7, 2018

nabobalis Mar 7, 2018

Cadair commented Mar 7, 2018

Replaces astropy-helper with sphinx-astropy #2494

Replaces astropy-helper with sphinx-astropy #2494

Conversation

yashkgp commented Mar 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nabobalis commented Mar 2, 2018 • edited

Cadair commented Mar 2, 2018

Choose a reason for hiding this comment

yashkgp Mar 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cadair commented Mar 2, 2018

yashkgp commented Mar 2, 2018

Choose a reason for hiding this comment

Cadair left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cadair commented Mar 7, 2018

nabobalis commented Mar 2, 2018 •

edited

yashkgp Mar 2, 2018 •

edited