Remove all sphinx components

[astrofrog]

Overview

This is a more extensive alternative to #367, and it:

• Removes the theme which is in https://github.com/astropy/astropy-sphinx-theme (and on PyPI)
• Removes the bundled sphinx-automodapi and numpydoc
• Removes the rest of astropy_helpers.sphinx to https://github.com/astropy/sphinx-astropy

It does not remove build_docs though.

Note that the astropy-sphinx-theme and sphinx-astropy repositories contain the full history of the files in those repos, including their origins in the astropy core package. I am publishing the scripts I used to do all this here: https://github.com/astrofrog/git-workflows (note that this isn't up to date yet as I have some more changes to push).

Note that sphinx-astropy is also a meta-package in that it includes dependencies on astropy-sphinx-theme, sphinx-automodapi, numpydoc, and sphinx-gallery. I haven't published sphinx-astropy yet as I want to make sure we have a chance to discuss this here first.

Backward-compatibility

These changes will require one change in astropy core and affiliated packages (when they update to astropy-helpers next), which to change this line:

https://github.com/astropy/astropy/blob/master/docs/conf.py#L48

from:

from astropy_helpers.sphinx.conf import *

to

from sphinx_astropy.conf import *

Note that we can make this change automatically when updating astropy-helpers. We could optionally add a nicer error message:

try:
    from sphinx_astropy.conf import *
except ImportError:
    print('Note that the documentation for this package requires the sphinx-astropy package to be installed')
    sys.exit(1)

This has the benefit of being super explicit in terms of what is required, which is an improvement over just the logging warning in #367.

See astropy/astropy#7139 for an example implementation in the core package.

I don't think we need to add a compatibility layer to astropy-helpers since in principle packages will be able to update their conf.py at the same time as the astropy-helpers submodule.

Steps forward

If we agree on this, these are the steps ahead:

• Merge Added package layout sphinx-astropy#2
• Do a release of the sphinx-astropy package
• Update this PR to use the released sphinx-astropy package
• Determine if the Travis matrix can be simplified (fewer sphinx versions)
• Open a PR to astropy core to use sphinx-astropy instead of astropy-helpers in conf.py, and mention dependency on sphinx-astropy to build the docs (Use sphinx-astropy to build the docs astropy#7139)
• Open a PR to the package template to use sphinx-astropy instead of astropy-helpers in conf.py

Fixes #338
Fixes #339

The following issue should probably be moved to sphinx-astropy, or at least resolved there then closed here: #270, #195, #211, #233, #320, https://github.com/astropy/astropy-helpers/issues/327

The following issue should probably be in sphinx-automodapi; https://github.com/astropy/astropy-helpers/issues/276

[astropy-bot]

Hi there @astrofrog 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

Everything looks good from my point of view! 👍

If there are any issues with this message, please report them here.

[bsipocz]

Open a PR to the package template to use sphinx-astropy instead of astropy-helpers in conf.py

@astrofrog - Just to double check, to use sphinx-astropy a package doesn't have to use this 3.1 version of the helpers, right?

[bsipocz]

once installed or once released?

[astrofrog]

Yep, good catch! I'll fix it before we merge.

[bsipocz]

Looks all good to me!

[bsipocz]

please double backtick the package names

[bsipocz]

what prompted this change?

[astrofrog]

I think this was actually related to trying to build the docs with python setup.py build_docs --use-system-astropy-helpers so isn't quite related to this PR but I found it while trying to use this PR

[bsipocz]

I really like this refactoring, but have a few questions left.

Do we plan to pin sphinx-astropy's version in the helpers releases? I just wonder about scenarios when it's not safe or desired by affiliated packages to upgrade to the latest sphinx-astropy. Since having this factored out, the overall complexity of the helpers package hierarchy is growing that may cause further confusion.

[astrofrog]

@astrofrog - Just to double check, to use sphinx-astropy a package doesn't have to use this 3.1 version of the helpers, right?

No, you can use sphinx-astropy without ever installing the helpers.

Do we plan to pin sphinx-astropy's version in the helpers releases? I just wonder about scenarios when it's not safe or desired by affiliated packages to upgrade to the latest sphinx-astropy. Since having this factored out, the overall complexity of the helpers package hierarchy is growing that may cause further confusion.

As far as I know there is no way to 'pin' sphinx-helpers in astropy-helpers as such, since astropy-helpers is never actually installed (so install_requires is not used). What we'll need to do is add sphinx-astropy to the .travis.yml of packages that test the docs build, and that's where the pinning would need to happen. But I'm not sure if we need to pin it, no more than we need to pin e.g. numpydoc or sphinx-automodapi. The great thing about having sphinx-astropy as a standalone package is we can easily do quick bugfix releases if downstream issues occur.

the overall complexity of the helpers package hierarchy is growing that may cause further confusion.

Just to be clear, astropy-helpers basically knows nothing about sphinx-astropy, and you can use astropy-helpers without using sphinx-astropy (if you don't want the default astropy config) and vice-versa you can use sphinx-astropy and just build the docs with make html. So in that sense there is no 'hierarchy' and fixing sphinx issues is going to be simpler than it used to be.

[bsipocz]

But I'm not sure if we need to pin it, no more than we need to pin e.g. numpydoc or sphinx-automodapi. The great thing about having sphinx-astropy as a standalone package is we can easily do quick bugfix releases if downstream issues occur.

But those were bundled into the helpers so far, so we had some control over what to include. You're probably right though that we can have a hot fix when any issues come up. I would just like to avoid situations when the random xy package start complaining with some weird looking error that comes up at a random occasion and the solution will be that we have updated something in sphinx-astropy (but since the packages are low traffic ones, that update might have happened a while ago).

[astrofrog]

This does raise a good point, which might have come up anyway even without this split - what if we want to change the default configuration some day? One solution since we are updating the imports anyway would be to update to versioned imports:

from sphinx_astropy.conf.v1 import *

this then allows us to make significant opt-in changes to the default configuration if we want in future (without requiring packages pin to a specific version) though we can just make changes to v1 if we want to roll it out to all packages.

[bsipocz]

Yes, the approach could work, and with that we may also want to consider to backport this refactoring to the 2.0.x branch, so anyone still supporting python2 could use sphinx-astropy.

Or it's already python3 only?

[astrofrog]

Yes, the approach could work, and with that we may also want to consider to backport this refactoring to the 2.0.x branch, so anyone still supporting python2 could use sphinx-astropy.

Anyone can use sphinx-astropy irrespective of astropy-helpers version. This PR just removes all the code from astropy-helpers but that's just cleanup - you can use sphinx-astropy even if you are on v2.0.x, except if sphinx-astropy isn't compatible with Python 2 (which I need to check).

Basically if you import sphinx_astropy.conf, then the code in astropy-helpers gets ignored.

[eteq]

Generally I'm 👍 to this, @astrofrog. However, two concerns:

First: Am I understanding right that you are not then planning to package all the new "requirements" as helpers dependencies? If so that seems problematic because one of the main selling points of the helpers is that it's a "one stop shop" and the user of it needn't worry too much about managing all their dependencies. Is there any reason not to have the helpers setup.py now have a requires=[..., 'sphinx-astropy']?

Second:

These changes will require one change in astropy core and affiliated packages

Why not, at least for a time, leave the existing import in place but have it simply be a placeholder astropy_helpers/sphinx/conf.py that does

from sphinx_astropy.conf import * 

warn(DeprecationWarning('Need to do the new fancy helpers thingie!'))

Or similar? That lets helpers users (i.e. affiliated package devs) know that this is coming, but gives them a window to do the helpes update without it breaking immediately?

[bsipocz]

Or similar? That lets helpers users (i.e. affiliated package devs) know that this is coming, but gives them a window to do the helpes update without it breaking immediately?

we can do that deprecation you suggest, but I would rather include the conf change in the same automated PR that updates for the 3.1 helpers that raising any deprecation warnings to the users. On the other hand, not deprecating but having that import in the helpers sounds like a good solution to me, too.

[astrofrog]

First: Am I understanding right that you are not then planning to package all the new "requirements" as helpers dependencies? If so that seems problematic because one of the main selling points of the helpers is that it's a "one stop shop" and the user of it needn't worry too much about managing all their dependencies. Is there any reason not to have the helpers setup.py now have a requires=[..., 'sphinx-astropy']?

We can add it to install_requires - however note that for packages that make use of the astropy-helpers as a submodule this might not be honored because then the helpers are not really installed as such. But I can definitely add it for cases where it is installed.

There are a couple of options here as far as I can see - we could either modify build_docs to automatically install sphinx-astropy temporarily when running build_docs (like we do for pytest-astropy when running the tests) or we could modify ah_bootstrap.py to automatically install sphinx-astropy also temporarily while running setup commands (e.g. build_docs). I can investigate both options to see what would work.

Why not, at least for a time, leave the existing import in place but have it simply be a placeholder astropy_helpers/sphinx/conf.py

Sounds good, that's easy to do!

[astrofrog]

@eteq - I've implemented the auto-install and the warning. The auto-install can't be tested completely yet as I need to do a release of sphinx-astropy first, but I tested it by using another package. Would something like this work for you? (this is like what we do for running tests). If you are happy with it I could do a release of sphinx-astropy to check it works properly?

[eteq]

Minor suggestion, but other than that this LGTM. I'm a bit concerned the auto-installer trick will confuse some people about where their sphinx is coming from... but such is life - it's probably no more (and probably less) complicated than the helpers install process, anyway...

[eteq]

(minor) maybe "use sphinx_astropy.conf in the sphinx-astropy package"? Just so a random user who pays no attention this stuff knows what to google...

[eteq]

One thought which is not strictly related to this PR: should we update the astropy docs to explicitly say somewhere "got to the sphinx-astropy repo to get help/report issues/etc on the sphinx machinery"?

[astrofrog]

@eteq - thanks for reviewing! Just a few things/clarifications based on your comments here and on slack:

• This will not auto-install Sphinx because Sphinx itself has to be installed in the first place for build_docs to even run:

$ python setup.py build_docs --use-system-astropy-helpers
running build_docs
error: Sphinx and its dependencies must be installed for build_docs.

This is the same as before.

• What gets auto-installed is sphinx-astropy and its dependencies - these get put inside an .eggs directory. However, they only get installed if they are not available at the system level. Furthermore, they are not available if one tries to 'import sphinx_astropy' outside of the docs build in the astropy repository as the eggs need to be explicitly added to the path. So this is completely seamless to the user and doesn't inject anything into their Python environment outside of the build.

• There is a use case I hadn't considered, which is that since we don't remove the egg files, if the user runs the docs build, then installs a specific version of sphinx-astropy at the system level then tries to run the docs build again, the egg will be used due to the sys.path.insert. The correct fix as you mentioned on slack is to actually use sys.append, so I'll fix this now.

• The fact that sphinx-astropy etc are auto-installed will be clear from the docs build log:

Searching for sphinx-astropy
Reading https://pypi.python.org/simple/sphinx-astropy/
Downloading https://pypi.python.org/packages/9e/82/9759758a1276f0d52cc0518e8db5f5ca1029ea0d5579d60fca52fb8f3a4a/sphinx_astropy-1.0-py2.py3-none-any.whl#md5=a5540d72c16e3e800dc44d2ec3d0a234
Best match: sphinx-astropy 1.0
Processing sphinx_astropy-1.0-py2.py3-none-any.whl
Installing sphinx_astropy-1.0-py2.py3-none-any.whl to /Users/tom/Dropbox/Code/Astropy/astropy/.eggs
writing requirements to /Users/tom/Dropbox/Code/Astropy/astropy/.eggs/sphinx_astropy-1.0-py3.6.egg/EGG-INFO/requires.txt

Installed /Users/tom/Dropbox/Code/Astropy/astropy/.eggs/sphinx_astropy-1.0-py3.6.egg
Searching for sphinx-automodapi
Reading https://pypi.python.org/simple/sphinx-automodapi/
Downloading https://pypi.python.org/packages/c6/be/7b24d806568a76d839f4ae0e84edcb71b7e5aa0f958d5bf35ec76acad648/sphinx_automodapi-0.7-py2.py3-none-any.whl#md5=8a2d97e446eba985db0dd13d60c57015
Best match: sphinx-automodapi 0.7
Processing sphinx_automodapi-0.7-py2.py3-none-any.whl
Installing sphinx_automodapi-0.7-py2.py3-none-any.whl to /Users/tom/Dropbox/Code/Astropy/astropy/.eggs
writing requirements to /Users/tom/Dropbox/Code/Astropy/astropy/.eggs/sphinx_automodapi-0.7-py3.6.egg/EGG-INFO/requires.txt

Installed /Users/tom/Dropbox/Code/Astropy/astropy/.eggs/sphinx_automodapi-0.7-py3.6.egg
Running Sphinx v1.7.0
loading pickled environment... not yet created
loading intersphinx inventory from https://docs.python.org/3/objects.inv...
loading intersphinx inventory from /Users/tom/Dropbox/Code/Astropy/astropy/.eggs/sphinx_astropy-1.0-py3.6.egg/sphinx_astropy/local/python3_local_links.

So if users do run into any issues, all the information should be there to figure out what might be going on with eggs vs system installs.

[astrofrog]

One thought which is not strictly related to this PR: should we update the astropy docs to explicitly say somewhere "got to the sphinx-astropy repo to get help/report issues/etc on the sphinx machinery"?

I have mixed feelings about this - if there are issues with building the astropy docs, it could be due to an issue in astropy itself, the helpers, or any of the extensions, or sphinx itself. People who understand the build process will likely report issues in the right place, and otherwise for users I think it's fine if they just open an issue in the main repository and we can dispatch as needed.

[eteq]

This will not auto-install Sphinx because Sphinx itself has to be installed in the first place for build_docs to even run

That was a typo, my bad - I meant the sphinx plugin installs. So I'm happy as far as that is concerned.

I have mixed feelings about this - if there are issues with building the astropy docs, it could be due to an issue in astropy itself, the helpers, or any of the extensions, or sphinx itself. People who understand the build process will likely report issues in the right place, and otherwise for users I think it's fine if they just open an issue in the main repository and we can dispatch as needed.

Hmm, that worries me for us because it's hard to keep a 30,000 foot view. And I think some users get turned off by the "thanks for your input, but that should go in the astropy-sphinx tracker, so I'm closing this and moving it there. Mostly just thinking aloud though - I think your answer here is reasonable and there isn't an obvious better solution...

Anyway, all of these are not issues, so I'm going to go ahead and merge.