issue_comments

Unit Test Results

6 files           6 suites   50m 26s :stopwatch: 16 200 tests 14 465 :heavy_check_mark: 1 735 :zzz: 0 :x: 90 396 runs  82 221 :heavy_check_mark: 8 175 :zzz: 0 :x:

Results for commit 1dfad9fa.

:recycle: This comment has been updated with latest results.

Hello @Illviljan! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2021-07-24 22:33:10 UTC

I think this is ready now.

I'm not super happy with the docs, I think copying makes sense but it doesn't match completely at the moment. I was thinking that can be solved by rewriting the dataarray version in a smarter way.

the reason for the docs failure is a intersphinx issue. We fixed that some time ago so you should be able to merge in master to get the build to pass.

I think making the dataset plots a thin wrapper for the dataarray plot functions is the way to go. I think most things for the lineplot works as intended now. I'm not sure about the decorator though. It's a bit too simple compared to _dsplot at the moment.

Supporting linewidth in plots should be possible but this is for a future pr I think. I did a small proof of concept here: ```python def _lineplot(ds, x, y, hue, linewidth): if len(ds[y].dims) > 3: raise NotImplementedError("too many dims.")

fig, ax = plt.subplots(1, 1)
for i, width in enumerate(ds[linewidth]):
    # Filter along linewidth:
    dsi = ds.isel(**{linewidth: i})

    # Values to plot:
    xplt = dsi[x]

    # if xplt has no dims:
    (xdim,) = xplt.dims
    (huedim,) = dsi[hue].dims
    yplt = dsi[y].transpose(..., xdim, huedim)

    # Set plot properties:
    len_lines = len(ds[hue])
    cmap = plt.get_cmap("viridis", len_lines)
    colors = plt.cycler(color=cmap(np.arange(len_lines)))
    lw = plt.cycler(lw=[1 + i * 3])
    ax.set_prop_cycle(colors * lw)

    # p = xr.broadcast(xplt, yplt)
    plt.plot(xplt, yplt, label=width.values)
    plt.legend()

# ax.plot doesn't return a mappable that fig.colorbar can parse. Create
# one and return that one instead:
norm = plt.Normalize(vmin=ds[hue].min(), vmax=ds[hue].max())
primitive = plt.cm.ScalarMappable(cmap=plt.get_cmap("viridis"), norm=norm)
fig.colorbar(mappable=primitive)

return fig, ax

ds = xr.tutorial.scatter_example_dataset() ds1 = ds.sel(z=0) _lineplot(ds=ds1, x="y", y="A", hue="w", linewidth="x") ```

Added a variant using dataarray, it's much cleaner although the wrappers had to be redone. I think we can move ds.plot.scatter to the dataarray side as well.

I wonder if it would be easier to construct a DataArray and then use DataArray.plot.line so everything is going through the same code path

Not really. It just looked very drafty ;-)

My inclination would be to not allow setting lw and ms via a variable or coordinates - you should then probably use a scatterplot. That would also take care of your 4th point.

hue only works with coordinates.

I think that's fine. It's always possible to extend later.

I've added a little todo list of things I've noticed, @mathause. Did you have something else in mind?

I wouldn't mind some more examples and matplotlib solutions if anyone has done any nice looking line plots with xarray before.

This looks like a good start but is also still very preliminary so I converted to a "work in progress".