Thanks!

A lot of doc build warnings. You might want to build the docs locally and ensure warnings are all gone before pushing out more commits.

Also tl;dr the original discussions: Does this solution depend on certain versions of Numpy?

Also tl;dr the original discussions: Does this solution depend on certain versions of Numpy?

@saimn might know better. I'm looking at numpydoc, and it doesn't appear to depend on a specific numpy version. Presumably reference links to numpy objects depend on the numpy version, but more generally, I think not (?).

This extension requires Python 3.5+, sphinx 1.6.5+ and is available from:

numpydoc on PyPI
numpydoc on GitHub
‘numpydoc’ should be added to the extensions option in your Sphinx conf.py. (Note that sphinx.ext.autosummary will automatically be loaded as well.)

Yes, I don't think there is a strong requirement on a given Numpy version.

I don't think there should be such a huge list of terms in numpydoc_xref_ignore. It seems the warnings are triggered by malformed docstrings, which would need to be fixed (but this may be a huge work :( ).

Example of a problematic docstring:

@mhvk @saimn
Got it working!
Setting this up highlighted that the docs are not currently super careful in the types description of parameters / returns, so the numpydoc_xref_ignore is quite long. A good ongoing project would be to slowly reduce this set by bringing the docs into alignment.

Also, the description of unit_like needs to be longer and more descriptive, like the array_like description in https://raw.githubusercontent.com/numpy/numpy/68b06ce36f7d9201481b73c150e2ccb3b5aa9540/doc/source/glossary.rst
I just dropped in a short placeholder sentence as a test. Any suggestions for content and examples would be appreciated.

Lastly, I haven't yet started combing the docs for places to swap in unit-like. It's a pretty Herculean endeavor given the size of the code-base and the ubiquity of units.

But it works!

I implemented an xref for Quantity.
Doing a global replace on : :class:`~astropy.units.Quantity` -> : Quantity affects 31 files across all modules in Astropy. OK to push?

Edit: undid (see #11118 (comment))

• I don't think we should replace things like `~astropy.units.Quantity`. It useful to have the full import name when reading the docstrings, to know how to import the class. (That's why I avoid references without the module name like `Quantity` which would work as well if Quantity is imported in the module)
• As I said above, we should fix the docstrings to avoid blacklisting such a huge list of words. On the project where I used this I have only:

numpydoc_xref_ignore = {'type', 'optional', 'default', 'or', 'of', 'method'}

• Some warnings could be fixed by defining useful aliases, e.g. for iterable, file_like, array_like etc. (See examples in https://github.com/musevlt/mpdaf/blob/d04c8674fdcaef01ab2900be569d8e543fb964ed/doc/conf.py#L88-L101)

I don't think we should replace things like ~astropy.units.Quantity.

Sure thing. I undid it.

we should fix the docstrings to avoid blacklisting such a huge list of words.

I agree. For a different PR or this one?

Some warnings could be fixed by defining useful aliases, e.g. for iterable, file_like, array_like

👍 . Though only file_like appears in the current numpydoc_xref_ignore, so the rest would require some docstring sleuthing. For a different PR or this one?

Also, I may be wrong, but I think array_like comes prepackaged. From https://numpydoc.readthedocs.io/en/latest/install.html#sphinx-config-options

The default numpydoc_xref_aliases will supply some common Python standard library and NumPy names for you. 

@mhvk I think this initial PR is ready for review.
As @saimn and I noted, there will need to be a fair bit of work to pare down numpydoc_xref_ignore, but I think that is for followup PRs.

Thanks for the great work on this @nstarman ! I missed that numpydoc has some default aliases (see below for the list, maybe add a link in our conf file ?), so that's great.
I still think we should try to reduce the number of warnings first, to reduce the number of terms in numpydoc_xref_ignore. Otherwise we may just forget about it if everything works ;). I'm happy to have a look in the coming days (hopefully) to fix the ones I can, and it would be great if a few other maintainers can do the same (fixing some docstrings requires a good knowledge of the sub-package).

https://github.com/numpy/numpydoc/blob/b352cd7635f2ea7748722f410a31f937d92545cc/numpydoc/xref.py#L62-L94

I would not worry too much about making the description lengthy for Unit-like - my slightly revised text has a link to Unit that should do the trick.

👍

On the warnings: is there a log where we can see where the errors occurred?

I'm attaching the log file from commenting out numpydoc_xref_ignore except for optional, or, of, instance, subclass
it is really spread throughout the package, but as it's just docstrings it's safe to edit, right? just a lot of docstrings to change.

build_docs.log

Also, elsewhere there was a question about the impact on build time: does this make building the docs substantially slower?

I haven't noticed any slowdown. I haven't done a timed test... but it's doesn't feel slower.

numpydoc has some default aliases (see below for the list, maybe add a link in our conf file ?)

Good idea. I'll add a link

I still think we should try to reduce the number of warnings first, to reduce the number of terms in numpydoc_xref_ignore. Otherwise we may just forget about it if everything works ;).

Haha. Definitely a possibility.

@mhvk @saimn

I've made some progress, but there' still a lot of work remaining. I think the various package managers need to take a look to confirm the changed docstrings are correct.

Also, I'm currently contending with some strange errors about not finding reference targets for things that should be there.

@mhvk @saimn
It's been a while, but I returned to this and have basically completed the x-refs. The numpydoc_xref_ignore is pretty minimal and I think this is ready for review. There are 112 impacted files and I think this will require all the various package managers to take a look to confirm the changed docstrings are correct.

Before anything else, this is fantastic. And clearly an amazing amount of work going through all bloody docstrings. Wow!!!

Thanks!

I do worry about how to proceed here, without getting into endless rebases, losing track of what has been reviewed and what not, while discussing possible changes to the terms, etc.

I guess it is difficult to go piecemeal because then some sections have errors? Is there any way to ignore errors in particular submodules, so that we can go step by step? (E.g., could we start with just astropy.units?)

I agree this will be a bit challenging. Unfortunately there is not a way to ignore errors in particular submodules. What we had before was a pile of ignores, but that had the problem that no one would get around to fixing them all if we committed just the stuff for units. Which is definitely true. The alternative, done here, has the problem of size and difficulty of review. Thankfully none of this is a code change that effects actual code performance. But it is a sizable impact on the docs.

Agreed with Marten, this is a really impressive work !
I'm having a first look and doing a few comments, and will try to continue the review later. I'm also a bit worried about the potential conflicts but I'm not sure what can be done for this (well, one thing, please avoid any change unrelated to docstrings ;)).

I think this is ready! One last call for comments, but I suggest we merge today.

@pllim is there a way to retrigger RTD? seems stuck. Edit: again.

Re: RTD -- Unfortunately, not. Can't skip either. You just have to wait a bit and push again or close/re-open.

As I said on #11590, most of the changes from #11590 are generated via this tool so can easily be reapplied on top of this pr which I partially did on nstarman#2; it spotted a number of typo/issues. I'm also working on https://github.com/Carreau/papyri to provide the same feature as sphinx when using terminal IPython or Jupyter.

It seem to (mostly) work, see below gif, with a couple limitations:

I thought it might be of interest to you to see how it looks like, as usually markup-heavy docstrings can be ugly in Jupyter, and I would be happy if you want to fill bugs there to tell me which sphinx features are missing in terminal rendering.

As I said on #11590, most of the changes from #11590 are generated via this tool so can easily be reapplied on top of this pr which I partially did on nstarman#2; it spotted a number of typo/issues. I'm also working on https://github.com/Carreau/papyri to provide the same feature as sphinx when using terminal IPython or Jupyter.

@Carreau Those both seem like really cool tools! Many of us, and affiliate packages, and 🤞 astropy, use pre-commit. Velin seems well suited for that (if it can be run on diffs) as part of a good development workflow. Additionally / alternatively (@pllim), this might be useful in the daily or weekly cron jobs.

Thanks @nstarman for your patience and determination here 😄 -- I think it's about time to merge this, and handle further changes in follow-ups. Any last minute comments here? Otherwise, I vote to merge!

Hey, I'm going to press that lovely green button! Thank so much, @nstarman, for taking this on (and, even more impressively, sticking with it - 486 items in the conversation...)

Thanks everyone for the numerous reviews and detailed discussion! In particular, thanks @mhvk @saimn @adrn for helping get this started and seen to completion. 🎉