issue_comments

Hi @johnomotani . Probably I have no time to finish this up and this is already too old. It would be nice if someone can update this PR.

@shoyer Sorry. My example was still not good (I modified the previous example slightly).

I think my current implementation does not work even for (the future version of) .sel, because the resultant xr.Dataset has the index for 'y' and labels for 'x'.

As you suggested, it looks more xarray-like if it returns coordinates for both 'x' and 'y', python <xarray.Dataset> Dimensions: (x: 3) Coordinates: * x (x) <U1 'c' 'b' 'a' Data variables: y (x) float64 1.4 4.3 1.4 so that it can be passed to .sel.

This API looks much more natural to me than the current one, although it has to wait for #974. I will update this PR.

For the function name. Based on your suggestion, how about labels_min or argmin_labels? I think function names of idxmin and (the current) indexes_min can be too different because idxmin should return 'index' while indexes_min returns label.

@shoyer Thanks for the comments.

• APIs
  Sounds reasonable suggestions. I renamed argmin_indexs -> indexes_min. Also I added idxmin that works as similar to pandas's idxmin. The APIs are
  python def indexes_min(self, dims=None, skipna=True): and python def idxmin(self, dim=None, skipna=True, keep_dims=False): (I found keep_dims for indexes_min brings another confusion and omit from them).

• isel_points's issue Currently, indexes_min works as ```python In [1]: import xarray as xr ...: da = xr.DataArray([[1, 2], [51, 40], [5, 6]], ...: [('x', ['c', 'b', 'a']), ('y', [1.4, 4.3])]) ...: da ...: Out[1]: <xarray.DataArray (x: 3, y: 2)> array([[ 1, 2], [51, 40], [ 5, 6]]) Coordinates:

• x (x) <U1 'c' 'b' 'a'
• y (y) float64 1.4 4.3

In [2]: da.indexes_min(dims='y') Out[2]: <xarray.Dataset> Dimensions: (x: 3) Coordinates: * x (x) <U1 'c' 'b' 'a' Data variables: y (x) float64 0 1 0

```

However, arguments of isel_points should be like python <xarray.Dataset> Dimensions: (points: 3) Data variables: x (points) int64 0 1 2 y (points) int64 0 1 0

This behavior is more intuitive?

OK, I think I finally understand the nuance of the return value -- thanks for describing that fully for me.

In theory (after #974 is implemented), the current return value from indexes_min should work for indexing, e.g., ```

indexes = da.indexes_min(dims='y') indexes <xarray.Dataset> Dimensions: (x: 3) Coordinates: * x (x) <U1 'c' 'b' 'a' Data variables: y (x) int64 0 1 0

da.sel(x=indexes.x, y=indexes.y) # or ds.sel(**indexes) <xarray.DataArray (x: 3)> array([ 1, 40, 5]) Coordinates: * x (x) <U1 'c' 'b' 'a' ``` So maybe that is the right choice, though I'm not entirely certain yet.

Side note: I'm still not super happy with the names idxmin and indexes_min. They look too different for methods that are only a small variation on each other. Maybe idxmin_dataset or idxmin_dict?

A few quick thoughts on API design:

• The most similar pandas method is called idxmin. We may not want to use the exact same name here, but it's something to keep in mind.
• We might want two separate methods, one like this that returns an OrderedDict/Dataset and another that returns just one DataArray (for use when reducing over only one axis). I might pick idxmin and indexes_min.
• A keep_dims=True argument like numpy is a nice way to preserve dimensions if desired.
• I'm a little surprised that it doesn't work to unpack a Dataset with ** in isel_points -- in theory I think it should.