Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOC: Bump Python version on RTD build and fix failure #10524

Merged
merged 4 commits into from Jul 6, 2020

Conversation

pllim
Copy link
Member

@pllim pllim commented Jul 2, 2020

Description

RTD complained about astropy/table/table.py:docstring of astropy.table.TableColumns.keys::py:class reference target not found: a set-like object providing a view on D's keys on latest build even though the PR merged reported successful RTD build. Hoping that bumping the Python version will fix the problem if it is due to some incompatibilities with older dependencies that cannot be upgraded because Python being used is too old.

UPDATE: I think the failure is real. I think we inherited something from Python that doesn't play well with Sphinx.

Looks like there is also astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.clear::py:class reference target not found: None. Remove all items from D..

UPDATE: Comparing the logs from last successful build and the failed build on master branch, I noticed the following:

UPDATE: Tried to pin using docs/requirements.txt and have it installed via .readthedocs.yml before editable install part but didn't work. A later download somehow upgraded it back to 1.1.0.

TODO:

@pllim pllim added this to the v4.2 milestone Jul 2, 2020
@pllim pllim changed the title DOC: Bump Python version on RTD build DOC: Bump Python version on RTD build and fix failure Jul 2, 2020
@pllim

This comment has been minimized.

1 similar comment
@pllim
Copy link
Member Author

pllim commented Jul 2, 2020

Now it is astropy/wcs/__init__.py:docstring of astropy.wcs.Wcsprm.get_cdelt::py:class reference target not found: double array[naxis]. Oh gosh, I hope I don't have to go patch CPython.

@pllim

This comment has been minimized.

@pllim
Copy link
Member Author

pllim commented Jul 3, 2020

Will switching from numpydoc to napoleon fix this? Has been on our wish list over at astropy/sphinx-astropy#5 for a while now.

UPDATE: Wow, that opened a different can of worms. Maybe similar to problem at scipy/scipy#3915 .
astropy/coordinates/baseframe.py:docstring of astropy.coordinates.BaseCoordinateFrame:42:Inline strong start-string without end-string

@pllim
Copy link
Member Author

pllim commented Jul 3, 2020

Found some interesting tidbits at https://stackoverflow.com/questions/11417221/sphinx-autodoc-gives-warning-pyclass-reference-target-not-found-type-warning but doesn't seem to help much in this case.

@pllim
Copy link
Member Author

pllim commented Jul 3, 2020

Well, the switch to napoleon didn't work out, but after getting through some hurdle to build doc locally, here is a full list of warnings that RTD will encounter with the current setup on master:

astropy/table/table.py:docstring of astropy.table.TableColumns.keys:: WARNING: py:class reference target not found: a set-like object providing a view on D's keys
astropy/table/table.py:docstring of astropy.table.TableColumns.values:: WARNING: py:class reference target not found: an object providing a view on D's values
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.clear:: WARNING: py:class reference target not found: None.  Remove all items from D.
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.copy:: WARNING: py:class reference target not found: a shallow copy of D
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.items:: WARNING: py:class reference target not found: a set-like object providing a view on D's items
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.keys:: WARNING: py:class reference target not found: a set-like object providing a view on D's keys
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.pop:: WARNING: py:class reference target not found: v, remove specified key and return the corresponding value.
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.update:: WARNING: py:class reference target not found: None.  Update D from dict/iterable E and F.
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.values:: WARNING: py:class reference target not found: an object providing a view on D's values
astropy/wcs/__init__.py:docstring of astropy.wcs.Wcsprm.get_cdelt:: WARNING: py:class reference target not found: double array[naxis]
astropy/wcs/__init__.py:docstring of astropy.wcs.Wcsprm.get_pc:: WARNING: py:class reference target not found: double array[naxis][naxis]
astropy/wcs/__init__.py:docstring of astropy.wcs.Wcsprm.get_ps:: WARNING: py:class reference target not found: list of tuples
astropy/wcs/__init__.py:docstring of astropy.wcs.Wcsprm.get_pv:: WARNING: py:class reference target not found: list of tuples

Aha! numpydoc 1.1.0 broke our RTD job.

@pllim pllim force-pushed the bump-rtd-python branch 2 times, most recently from 913ad89 to 2960c82 Compare July 3, 2020 02:09
@pllim pllim added 🔥 Critical and removed table timeseries labels Jul 3, 2020
@pllim pllim requested a review from astrofrog July 3, 2020 02:34
@pllim
Copy link
Member Author

pllim commented Jul 3, 2020

The RTD failure is going to bite unrelated PRs, so better get this in sooner than later.

Copy link
Member

@mwcraig mwcraig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, thanks for tracking down the issue! Should we open an issue as a reminder to change the version restriction when there is a new numpydoc release?

@mhvk
Copy link
Contributor

mhvk commented Jul 4, 2020

@pllim - wow, that was good that I saw this, as I was getting quite stuck with similar warnings on my own package. What a mess! I had thought this could be solved with https://github.com/astropy/astropy/blob/master/docs/nitpick-exceptions, but that doesn't work for these errors.

Aside: for myself, I added a --keep-going flag to sphinx-build so that at least I get all errors in one go. Though it means one has to &^$&#( scroll up again to actually find the warnings.

@mhvk
Copy link
Contributor

mhvk commented Jul 4, 2020

p.s. I now see our very own Erik pointed to this file in the answer on the stack-overflow question you pointed to... Still not sure why it doesn't seem to work here.

Copy link
Member

@Cadair Cadair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the more reason to drop numpydoc in favour of napoleon.

@astrofrog
Copy link
Member

Note about the current warnings:

astropy/table/table.py:docstring of astropy.table.TableColumns.keys:: WARNING: py:class reference target not found: a set-like object providing a view on D's keys
astropy/table/table.py:docstring of astropy.table.TableColumns.values:: WARNING: py:class reference target not found: an object providing a view on D's values
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.clear:: WARNING: py:class reference target not found: None.  Remove all items from D.
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.copy:: WARNING: py:class reference target not found: a shallow copy of D
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.items:: WARNING: py:class reference target not found: a set-like object providing a view on D's items
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.keys:: WARNING: py:class reference target not found: a set-like object providing a view on D's keys
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.pop:: WARNING: py:class reference target not found: v, remove specified key and return the corresponding value.
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.update:: WARNING: py:class reference target not found: None.  Update D from dict/iterable E and F.
astropy/timeseries/__init__.py:docstring of astropy.timeseries.BoxLeastSquaresResults.values:: WARNING: py:class reference target not found: an object providing a view on D's values

This kind of issue is quite common and due to the inherited-members directive here:

.. automodapi:: astropy.timeseries
   :inherited-members:

There are three solutions:

  • Overload the above methods and write new docstrings
  • Add a skip directive to the above automodapi directive to skip the BoxLeastSquaresResults class and add a separate entry that just documents that class, without the inherited members
  • Figure out how to get the nitpick exceptions working for this - it's possible that the presence of a quote (') in the warnings is making this not work properly, so we need to figure out how to escape it correctly, maybe by looking at the nitpick code
astropy/wcs/__init__.py:docstring of astropy.wcs.Wcsprm.get_cdelt:: WARNING: py:class reference target not found: double array[naxis]
astropy/wcs/__init__.py:docstring of astropy.wcs.Wcsprm.get_pc:: WARNING: py:class reference target not found: double array[naxis][naxis]

This is a real error - 'double array[naxis]' is not a real type, so this should be ``numpy.ndarray` and the description should mention any dimensionality restrictions.

astropy/wcs/__init__.py:docstring of astropy.wcs.Wcsprm.get_ps:: WARNING: py:class reference target not found: list of tuples
astropy/wcs/__init__.py:docstring of astropy.wcs.Wcsprm.get_pv:: WARNING: py:class reference target not found: list of tuples

Also a real error I think - should just say list and the description should say what should be in the list?

Switching to Napoleon would not fix this - napoleon doesn't really support the full numpydoc spec, see sphinx-doc/sphinx#6861 for an example of an issue I ran into.

@pllim
Copy link
Member Author

pllim commented Jul 6, 2020

Thanks for the inputs, everyone!

Re: three solutions -- I did start on first two and then started doubting the reality and rolled back. I mean, numpydoc 1.1.0 is a minor release, so why are things breaking so badly for us? Why didn't it complain until now? As for the third one, I wasn't ready to mess with @embray 's magic in case this breakage will be patched upstream by numpydoc.

Re: drop numpydoc in favour of napoleon -- It's not a bed of roses. I got thousands of warnings (like 3k-4k).

So how should I proceed? Should I merge this and open a follow-up issue like @mwcraig said?

@mhvk
Copy link
Contributor

mhvk commented Jul 6, 2020

My suggestion would be to do what you do right now, for the moment just fix the numpydoc version so tests pass again, and then raise another issue. I did get a sense over at the numpydoc issue that this might get fixed; or perhaps we try to find a fix ourselves so we can nit-pick.
Though ideally fix the "real" errors that @astrofrog pointed out. I think list of tuple may actually work, i.e., writing it as list of tuple*s* may be the problem!

@pllim
Copy link
Member Author

pllim commented Jul 6, 2020

What about now, @mhvk and @astrofrog ? Hope I didn't go too far with the search and replace.

Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems good enough for now!

@pllim
Copy link
Member Author

pllim commented Jul 6, 2020

Thanks for the review! I'm taking the liberty to merge now so this failure won't be a blocker for other PRs, especially given it's SciPy week.

@pllim pllim merged commit 3a8732f into astropy:master Jul 6, 2020
@pllim pllim deleted the bump-rtd-python branch July 6, 2020 17:23
@pllim
Copy link
Member Author

pllim commented Jul 6, 2020

To close the loop, follow-up issue is at #10529

@embray
Copy link
Member

embray commented Aug 3, 2020

As for the third one, I wasn't ready to mess with @embray 's magic in case this breakage will be patched upstream by numpydoc

Wait, what was my magic? I'm a little confused.

@pllim
Copy link
Member Author

pllim commented Aug 3, 2020

astrofrog pushed a commit that referenced this pull request Aug 6, 2020
DOC: Bump Python version on RTD build and fix failure
@astrofrog astrofrog modified the milestones: v4.2, v4.1 Aug 6, 2020
@astrofrog
Copy link
Member

I am backporting this to v4.1.x to fix some docs failures there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants