issue_comments

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)

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

@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!)

okay, go ahead

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.

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 🍻

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?

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!

:man_shrugging: I think it's fine to merge right now. This is great work!

Ok sounds good then

it will print all warnings, then return a non-zero error code (or zero if there was no warning). Of course, if there are actual errors, it will still fail (at least, that is what the help on sphinx-build says).

Will it still fail of there are errors?

re warnings checking: I'd start with something like sphinx-build -WT --keep-going and wait until we fixed all the nitpicky warnings to enable -n

I think so. Though would a decorator be cleaner? I'm pretty sure dask uses a decorator when they wrap numpy functions

Great! That's cool we can start checking warnings.

I think wrapping the numpy docstrings seems pretty reasonable—what are you concerns? Any thoughts @dcherian ?

re sections in whats-new.rst: done, if interp is indeed the label to the correct section (at least it creates a link)?

Looking at the links to h5netcdf and cftime again: while h5netcdf is only referenced once (h5netcdf.File in whats-new.rst), cftime.datetime is referenced quite often and not just in whats-new.rst. So I think it might be better to ask the cftime project to fix their intersphinx: bash $ python -m sphinx.ext.intersphinx https://unidata.github.io/cftime/objects.inv std:doc api API : api.html index cftime : index.html installing Installation : installing.html std:label genindex Index : genindex.html modindex Module Index : py-modindex.html search Search Page : search.html Edit: let's see what they think: unidata/cftime#132

I think I found the reasons for the numpy docstring warnings: 1. some of the numpy functions include signatures. I think that is because they are written in C? In the signature there is an asterisk which sphinx misinterprets. 2. the numpy docstrings are indented which somehow confuses sphinx, but only for some of the functions 3. some references are labeled, but not used

all of these can be silenced by rewriting the docstrings: ```python def _create_op(name): func = _UFuncDispatcher(name) func.name = name doc = textwrap.dedent(getattr(_np, name).doc) if doc.strip().startswith(name): start_location = doc.strip().find("\n\n") doc = doc[start_location + 2:]

for ref_num in range(5):
    if f".. [{ref_num}]" not in doc or f"[{ref_num}]_" in doc:
        continue

    doc = doc.replace(f".. [{ref_num}]", f"{ref_num}.    ")

func.__doc__ = (
    "xarray specific variant of numpy.%s. Handles "
    "xarray.Dataset, xarray.DataArray, xarray.Variable, "
    "numpy.ndarray and dask.array.Array objects with "
    "automatic dispatching.\n\n"
    "Documentation from numpy:\n\n%s" % (name, doc)
)
return func

``` However, I'm not sure this is a good idea.

I think interpolating values with interp never had an existing label,

Lets do :ref:interp or whatever works for references to interpolation.rst

copies vs views is still there but with underscores instead of spaces and faq.other_projects got moved/renamed to related-projects

Let's change these appropriately.

the entry for tutorial actually looks fine, I'd simply put it in double-backticks.

:+1:

now the only warnings left in non-nitpicky mode are the ones in the numpy docstrings and these: whats-new.rst:1268: WARNING: undefined label: interpolating values with interp (if the link has no caption the label must precede a section header) whats-new.rst:1387: WARNING: undefined label: faq.other_projects (if the link has no caption the label must precede a section header) whats-new.rst:3253: WARNING: undefined label: copies vs views (if the link has no caption the label must precede a section header) whats-new.rst:4035: WARNING: unknown document: tutorial which are all related to the documentation. What should we do with these? I think interpolating values with interp never had an existing label, copies vs views is still there but with underscores instead of spaces and faq.other_projects got moved/renamed to related-projects. tutorial got removed a long time ago. I'm not sure about the first three, but the entry for tutorial actually looks fine, I'd simply put it in double-backticks.

Edit: there are also 5 lines related to DataArray properties like xarray/doc/generated/xarray.DataArray.attrs.rst:2: WARNING: duplicate object description of xarray.DataArray.attrs, other instance in generated/xarray.DataArray, use :noindex: for one of them

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!

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?

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

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).

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.

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.

```

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)?

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?

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?

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

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: ```python

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,DataArrayGroupByexplicitly implementsquantilewhileDatasetGroupBy` 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.

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)

Awesome, thanks @keewis !!

I think the changes in https://github.com/pydata/xarray/commit/5d9d263e40e1f67910cbefaf96d46a91c560b8b5 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?