Missing dependencies for astropy docs build

taldcroft commented 5 years ago

I just did the experiment of following the instructions at: https://astropy.readthedocs.io/en/latest/install.html#building-documentation

These instructions are really pretty good (simple!) and seem most of the way to allowing newcomers to make and test documentation updates. But...

I made a clean conda python=3.7 environment and a fresh astropy clone and did:

conda install -c astropy sphinx-astropy
python setup.py build_docs

It looks like the following dependencies for building astropy docs are missed:

Cython (this gets pip installed, but still...)
pytest (build process exception ModuleNotFoundError: No module named 'pytest')
pyyaml (module not found exception)
scipy (build seemed to get stuck at some 95% reading source; when I ctrl-c'd at 25 CPU minutes it complained about scipy import errors)
skimage (After fixing the previous issues, I left the doc build going and stopped at 1.5 hrs CPU, and it showed an exception ModuleNotFoundError: No module named 'skimage')

bsipocz commented 5 years ago

A pip install -e .[docs] seems to be missing from that docs page.

bsipocz commented 5 years ago

(So I would rather call this an astropy documentation issue than anything wrong with sphinx-astropy)

taldcroft commented 5 years ago

That didn't fix things:

  542  conda create -n astropy-docs python
  543  source activate astropy-docs
  544  conda install -c astropy sphinx-astropy
  545  pip install -e .[docs]
  550  git clean -fxd
  556  python setup.py build_docs

(astropy-docs) neptune$ python setup.py build_docs
Searching for cython>=0.21
Best match: Cython 0.29.10
Processing Cython-0.29.10-py3.7-macosx-10.7-x86_64.egg

Using /Users/aldcroft/git/astropy-docs/.eggs/Cython-0.29.10-py3.7-macosx-10.7-x86_64.egg
running build_docs
running build
running build_py
...
loading intersphinx inventory from https://docs.scipy.org/doc/numpy/objects.inv...
loading intersphinx inventory from https://docs.scipy.org/doc/scipy/reference/objects.inv...
loading intersphinx inventory from https://matplotlib.org/objects.inv...
loading intersphinx inventory from http://docs.h5py.org/en/stable/objects.inv...
loading intersphinx inventory from https://docs.pytest.org/en/stable/objects.inv...
loading intersphinx inventory from https://ipython.readthedocs.io/en/stable/objects.inv...
loading intersphinx inventory from http://pandas.pydata.org/pandas-docs/stable/objects.inv...
loading intersphinx inventory from https://sphinx-automodapi.readthedocs.io/en/stable/objects.inv...
loading intersphinx inventory from http://docs.astropy.org/projects/package-template/en/latest/objects.inv...
[autosummary] generating autosummary for: changelog.rst, config/config_0_4_transition.rst, config/index.rst, constants/index.rst, convolution/index.rst, convolution/kernels.rst, convolution/non_normalized_kernels.rst, convolution/using.rst, coordinates/angles.rst, coordinates/apply_space_motion.rst, ..., whatsnew/1.0.rst, whatsnew/1.1.rst, whatsnew/1.2.rst, whatsnew/1.3.rst, whatsnew/2.0.rst, whatsnew/3.0.rst, whatsnew/3.1.rst, whatsnew/3.2.rst, whatsnew/4.0.rst, whatsnew/index.rst
Failed to import 'astropy.io.fits.scripts.fitscheck': no module named astropy.io.fits.scripts.fitscheck

Exception occurred:
  File "/Users/aldcroft/git/astropy-docs/build/lib.macosx-10.7-x86_64-3.7/astropy/tests/helper.py", line 12, in <module>
    import pytest
ModuleNotFoundError: No module named 'pytest'
The full traceback has been saved in /var/folders/_3/rmv7z1hx5jggk39p65j0dcxr0000gn/T/sphinx-err-d137ld34.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!
Sphinx Documentation subprocess failed with return code 2

bsipocz commented 5 years ago

hmm. My bad. It should be then pip install .[docs,all]

@astrofrog - Is there a way to make the [docs] tag install [all], too?

taldcroft commented 5 years ago

And I know it's falling out of fashion, but I still use conda mostly. Is it possible to have a turnkey solution that installs the dependencies as conda packages? (Understood that pip still works, it just isn't playing with the rest of my conda).

bsipocz commented 5 years ago

Afaik we don't have any. It's actually quite annoying, I fully agree, as the current setup doesn't allow an as easy pinning of versions as we used to have when we used requirements.txt files, etc. See https://github.com/astropy/astropy/issues/8820

I'm also cc-ing @Cadair and @astrofrog so they see going with the shiniest new ways is not always OK for all the use cases.

astrofrog commented 5 years ago

i don't think there's anything fundamentally wrong with the infrastructure here, but it is indeed a real issue that pip install .[docs] isn't sufficient - we should expand docs to include all actual required dependencies for the docs, or alternatively make it so all includes all docs dependencies too and remove all. I can investigate what exactly is needed later today.

@bsipocz - I don't think there's a way to make docs extras_requires pick up all automatically.

astrofrog commented 5 years ago

For getting this to work with conda, indeed the issue is that the extras-like syntax is not supported by conda (see https://github.com/conda/conda/issues/7502 for a discussion).

Adding back requirements files as well as a conda environment file is an option, of course, it's just a pain to have to list the dependencies in several places and keep them in sync.

bsipocz commented 5 years ago

Can we do a trickery that they are populated from one canonical list?

pllim commented 4 years ago

The pytest import is because fitscheck somehow using astropy.tests.helper.catch_warning and astropy.tests.helper imports pytest at the module level. I don't know why a convenience function that is not a test function uses a "test helper," but it is the most unfortunate. I tried replacing it with warnings.catch_warnings in astropy/astropy#10504 but it broke most spectacularly.

Ideally, doc build should not need pytest to be installed, so I would consider this a bug on the core astropy side.

pllim commented 4 years ago

@taldcroft , thank you so much for the clear listing. It helped me past astropy/sphinx-automodapi#108 .

saimn commented 4 years ago

I tried replacing it with warnings.catch_warnings in astropy/astropy#10504 but it broke most spectacularly.

I had a look, and that's a nice can of worms ;D. Basically Astropy's catch_warnings is overwriting the warnings defined in setup.cfg for pytest, by calling treat_deprecations_as_exceptions in its __exit__ method. So the global error clause in pytest's config was ignored once Astropy's catch_warnings was called...

https://github.com/astropy/astropy/blob/49708febbdaa1d1bf53f96981bf1f1d71295d33d/setup.cfg#L102-L103

When using warnings.catch_warnings this does not happen, so the global error clause is there, and the checksum verification in io.fits is issuing warning when the checksum is wrong, which turns into errors, which break the code's logic... Not sure what's the best solution here.

pllim commented 4 years ago

Thanks, @saimn ! Let's move the FITS convo to astropy/astropy#10528 .

astropy / sphinx-astropy

Missing dependencies for astropy docs build #21