astropy / astropy-helpers

Helpers for Astropy and Affiliated packages
BSD 3-Clause "New" or "Revised" License
28 stars 42 forks source link

Make sphinx-astropy an explicit dependency for docs build #472

Closed bsipocz closed 5 years ago

bsipocz commented 5 years ago

Packages relying on astropy-helpers to provide docs building infrastructure should make sphinx-astropy an explicit dependency. Being cleaver and building it and the full dependency three as eggs proved to be problematic when those dependencies start to introduce some version restrictions (e.g. mpl for python, and numpydoc for sphinx) See e.g., #462, #463, #465, etc.

And in addition building the eggs for all those dependencies is the least efficient way to install them (#471)

Now it is a good time to make this change, before the actual v3.2 release.

cc @astrofrog

astrofrog commented 5 years ago

I personally agree, I think it's reasonable to just crash and tell users that sphinx-astropy is needed if it's not installed.

pllim commented 5 years ago

Where exactly do you want that message? There is already this here

https://github.com/astropy/astropy-helpers/blob/222e6b5cdb62cdecf8221f3f4c70fa82beda83fe/astropy_helpers/sphinx/conf.py#L5

astrofrog commented 5 years ago

I think we should also raise an error here:

https://github.com/astropy/astropy-helpers/blob/master/astropy_helpers/commands/build_sphinx.py#L192

just to be on the safe side (instead of calling ensure_....

bsipocz commented 5 years ago

I'll open a PR for this at the end of the today.

saimn commented 5 years ago

And we also have now an explicit dependency with sphinx-asdf, so installing sphinx-astropy under the hood is not enough to build the docs anyway.

astrofrog commented 5 years ago

Arguably we could make sphinx-asdf a dependency of sphinx-astropy, but let's fix its major performance issues first!

bsipocz commented 5 years ago

This was really done for packages other than astropy who use the template and the helpers but not necessarily know what goes on under the hood. astropy always had the explicit dependency on sphinx-astropy listed in setup.py/setup.cfg.

bsipocz commented 5 years ago

Addressed by #474 (and now I realized "address" is not a magic word, so that PR didn't close this issue automatically).