Silence sphinx warnings

[keewis]

This PR tries to silence most sphinx warnings. 5d9d263 is a collection of changes that silence warnings, but they also reduce the readability of the plain text docstrings, so I'd especially appreciate feedback on these.

There are still some warnings left, due to various reasons:

• the ufunc warnings are inherited from numpy
• some are due to duplicate target names in whats-new.rst (e.g. CF Conventions but also developer names).
• two files are not included in any toctree (README.rst and api-hidden.rst)
• references in whats-new.rst to labels that were either renamed or removed
• conflicting definitions for properties
• the CI also lists a lot of broken references to other functions / methods, which do not show for me

To fix these, I would

• update the numpy docstrings upstream or find a way to ignore these warnings (probably both)
• reference both files from somewhere
• track down the labels the references refer to and add them back, or disable the links

but I have no idea how to deal with the duplicate target names, duplicate definitions of properties, where to put the references to both files or if it would be better to leave the broken references broken. I did not look at the long list of broken references the CI shows since I can't reproduce it.

• short-term fix for Hundreds of Sphinx errors #3370
• Passes black . && mypy . && flake8
• Fully documented, including whats-new.rst for all changes and api.rst for new API

[max-sixty]

Awesome, thanks @keewis !!

I think the changes in 5d9d263 are good (and having one standard is good for long-term readability)

For old whatsnew entries, I think it's fine that the references have decayed if we can silence the warnings (maybe easiest way is your suggestion to just remove the links, though)

Should we put an issue in for the numpy docstrings?

As a follow-up, is there a way to add a check in CI so these don't stack up again as people make changes? Or maybe difficult without stamping them all out?

[keewis]

CI: I think it's possible using sphinx-build -W --keep-going, which continues as normal and returns a non-zero error code if a warning was emitted, but first we need to stamp out all warnings (otherwise we have a lot of failing checks on unrelated PRs).

I also noticed the CI uses -n for a nit-picky mode that increases the number of emitted warnings, so now I know where the extra warnings come from.

numpy: sure, but updating upstream is only a long term fix, we will still have to somehow ignore these warnings if we want to check for new warnings using CI.

Edit: the warning about files not included by any toctree can be silenced by adding :orphan: to the top of the file (see documentation and this question on SO)

[keewis]

I took a shot at using :orphan:, but doc/README.rst seems like it should be excluded from the documentation?

Also, while trying to fix the nit-picky warnings, I found this:

>>> import xarray as xr
>>> ds = xr.Dataset(data_vars={"a": ("x", list("abcd"))}, coords={"x": range(4)})
>>> ds.a.groupby(ds.x % 2 == 0).quantile
<bound method DataArrayGroupBy.quantile of DataArrayGroupBy, grouped over 'x' 
2 groups with labels False, True.>
>>> ds.groupby(ds.x % 2 == 0).quantile
AttributeError: 'DatasetGroupBy' object has no attribute 'quantile'

despite the documentation claiming otherwise (and indeed, DataArrayGroupBy explicitly implements quantile while DatasetGroupBy doesn't). Does this deserve its own issue?

Implementing DatasetGroupBy.quantile probably won't silence the nit-picky warning, though, since DataArrayGroupBy.quantile has the same problem. I'll investigate more on this.

[max-sixty]

I took a shot at using :orphan:, but doc/README.rst seems like it should be excluded from the documentation?

Is doc/README.rst some special page we have to have? It seems to have no content value at the moment? Could we remove?

despite the documentation claiming otherwise (and indeed, DataArrayGroupBy explicitly implements quantile while DatasetGroupBy doesn't). Does this deserve its own issue?

Definitely, good spot

[keewis]

does anyone have an idea about what to do about broken links in whats-new.rst? There are also a lot of those that reference xray but apart from that would work fine. Since we don't want to update them, the easiest way to silence these warnings would be to remove any prefixes like :func: or :py:meth:. I can't seem to find anything on this, but ideally there would be a way to mark links as intentionally broken. Thoughts?

[max-sixty]

There are also a lot of those that reference xray but apart from that would work fine.

I think it'd be good to do a find/replace on those, if that solves our issues?

Or are there lots that remain despite that?

[keewis]

unfortunately, those are not the majority of warnings and this would also cause warnings for deprecated methods (like xray.Dataset.isel_points). Unless someone has a better idea, I'd vote for converting :py:meth:... links to double backtick-quoted text

I also created a preview page, but that currently fails. Can someone help me with the build errors (excessive memory consumption)?

[max-sixty]

Yes good idea @keewis

Where do you see the excessive memory consumption? When building locally?

I see the tests failing from:

Sphinx parallel build error:
RuntimeError: Non Expected warning in `/home/vsts/work/1/s/doc/plotting.rst` line 570
##[error]The operation was canceled.

[keewis]

the CI will probably work when pushing new commits (or one might try to remove -j auto so sphinx runs sequentially: this might take longer but should avoid timeouts).

No, I was trying to set up readthedocs for this branch which I thought could make discussing the changes a bit easier. However, the current build fails because conda ends up consuming too much memory. My impression was that this is a common problem that was somehow fixed for master.

[keewis]

for author names in whats-new.rst entries: would it be reasonable to always require a format of Name <https://github.com/user>? The warnings regarding these was due to inconsistencies with the link location (mostly a trailing slash or using http instead of https).

[max-sixty]

would it be reasonable to always require a format of Name <https://github.com/user>? The warnings regarding these was due to inconsistencies with the link location (mostly a trailing slash or using http instead of https).

For sure. How's your regex-fu for find-replacing? I can try and have a go otherwise

[keewis]

there were not enough entries to actually require regex-replace, so I did it by hand.

How would you handle links to h5netcdf and cftime? Convert them to double-backtick quoted? I don't think either of them has working intersphinx entries?

[dcherian]

How would you handle links to h5netcdf and cftime? Convert them to double-backtick quoted? I don't think either of them has working intersphinx entries?

Sounds good!

[keewis]

there are three warnings (locally for me five) of the same kind left before we could switch on -W --keep-going, but I don't know how to fix these:
autosummary (or autodoc?) seems to put labels for name, values, attrs (locally also coords and dims) in both xarray.DataArray.rst and xarray.DataArray.name.rst (or similar). The warnings recommend to put :noindex: in one of these, but both are generated so I don't know what to edit. Does anyone know more?

[dcherian]

🤷‍♂️ I think it's fine to merge right now. This is great work!

[max-sixty]

It would be awesome to be able to enable that @keewis, so we don't regress in future. We can merge this for now and come back to that?

I may have some time later this week to look more directly.

Great work, again!

[keewis]

Enabling is blocked by these warnings, so we can only do that once we silenced them. After that, I would be fine with a merge.

[dcherian]

I'm going to merge so we can get the docstring improvements out. Let's have a follow-up PR to fix the remaining warnings.

Thanks for the great work @keewis 🍻

[keewis]

okay, go ahead

[max-sixty]

@keewis do you have a twitter username and first name? We're going to start calling out big new contributors on our twitter and release notes. (@keewis is also OK if you prefer!)

[keewis]

no, sorry, I don't have a twitter account. Can you use the github handle?

[max-sixty]

Done! https://twitter.com/xarray_dev/status/1196963951340392448?s=20

(And in case others see this: we're going to be calling out a few others who have made big contributions recently too; we're really excited and appreciative of those who've recently made such prolific contributions)