built-in accessor documentation

[keewis]

We currently use property to attach our built-in accessors (plot, dt and str) to the DataArray and Dataset classes. However, because property returns itself when trying to access it as a class attribute, this does not work when generating the documentation (sphinx inspects classes, not class instances).

This adds a property-like descriptor that works on both classes and instance objects (the name could be more descriptive) and uses that to document the Dataset.plot.* and DataArray.plot.* methods (see the rendered documentation). I have not been able to get sphinx to work with _PlotMethods.__slots__, though.

A few questions / comments on this:

1. I noticed we have DataArray.plot.__call__ and xarray.plot.plot but not DataArray.plot.plot. Is that something that is worth adding?

2. The functions decorated with the custom property define docstrings which currently are lost. Should we patch them on their return values?

3. Right now, the error message when accidentally trying to call e.g. xr.DataArray.plot.line() is not very helpful:

AttributeError: 'NoneType' object has no attribute 'dims'

4. Now that we can document the accessors, we need to think about how to structure api.rst. For plot and str, we could just list methods / attributes in subsections of DataArray / Dataset (or keep the Plotting / str / ... sections and add class subsections?), but dt is more complicated since it dispatches for datetime and timedelta

• Closes Documentation for injected methods #3625
• Passes isort -rc . && black . && mypy . && flake8
• Fully documented, including whats-new.rst for all changes and api.rst for new API

Edit: err, it seems I used the branch I pushed to the main repository for a documentation preview for this...

[dcherian]

Excited to see this work at last.

[keewis]

there are a few of my questions / comments we can resolve by using something like

class UncachedAccessor:
    def __init__(self, accessor):
        self._accessor = accessor

    def __get__(self, obj, cls):
        if obj is None:
            return self._accessor

        return self._accessor(obj)

The only disadvantage is that now pydoc (or help()) show the accessor as a class. That was already the case for str and dt, but plot was a function.

I think we could try to trick inspect.getmembers (which is used to decide which kind of output should be done), but that would require a lot of monkeypatching I'd rather not do.

pandas simply uses the same docstring for the class and __call__, which is not ideal but a lot simpler.

[dcherian]

cc @crusaderky

[shoyer]

I think pandas might do something similar to do this? It might be worth checking, though it looks like you've figured most of it out already :)

[shoyer]

This is a really clever yet simple fix. I like it!

A few ideas for testing:

• Verify that DataArray.plot returns the PlotMethods object.
• Verify that data_array.plot.__doc__ is still there.

[shoyer]

there are a few of my questions / comments we can resolve by using something like

class UncachedAccessor:
    def __init__(self, accessor):
        self._accessor = accessor

    def __get__(self, obj, cls):
        if obj is None:
            return self._accessor

        return self._accessor(obj)

The only disadvantage is that now pydoc (or help()) show the accessor as a class. That was already the case for str and dt, but plot was a function.

I think we could try to trick inspect.getmembers (which is used to decide which kind of output should be done), but that would require a lot of monkeypatching I'd rather not do.

pandas simply uses the same docstring for the class and __call__, which is not ideal but a lot simpler.

👍 for a simple solution here like pandas rather than using inspect. I think it's probably actually slightly more accurate to show the class.

[keewis]

where should the docstring of the accessor come from? Right now, this is the docstring of the return value which seems reasonable to me.

I added tests verifying that cls.property returns the accessor class and obj.property the accessor instance. Do we need additional checks for __doc__?.

[keewis]

I added the accessor templates from pandas (we should eventually outsource these to a separate package used by both pandas and xarray) and modified the API page to display the accessor version of the plotting functions.

I think we should add a accessor section to each of the main objects (Dataset and DataArray), where we either copy what pandas does – listing every method / attribute of the accessor as a subsection of the class documentation – or we could show them as attributes (just like what we do for the injected numpy functions) and generate separate pages for the accessors.

[dcherian]

Thanks @keewis. LGTM and it's a very nice improvement to our documentation.

[keewis]

I think this is ready for merging; the fix of the DataArray.plot summary is going to be a new PR. So unless there are any objections I'm going to merge tomorrow.