issue_comments

Why would I have a xarray.core.dataset.DataVariables object as input?

Indeed, you would not. I think my earlier comment was a little confusing here. I meant you could have name(s) of variables in a Dataset (which means either coords or data_vars) or coords on a DataArray.

In my mind it should only be 1.) name(s) of existing index coords, or 2.) 1D DataArray(s) with dim in self.dims

Yes, agreed.

Sounds good. As I'm writing the type-checking code I run into this question: Why would I have a xarray.core.dataset.DataVariables object as input? A DataVariables object could contain multiple DataArrays, which makes the interpretation a bit unclear. In my mind it should only be 1.) name(s) of existing index coords, or 2.) 1D DataArray(s) with dim in self.dims

What would the signature of sortby() be then?

Maybe something like: sortby(variables, ascending=True), where variables can be any of: - name of a 1D variable in coords (on a DataArray) or coords/data_vars (on a Dataset): these get converted in a DataArray like self[name]. - a 1D DataArray, with a dimension found in self.dims - list of either of the above, either along the same or different dimensions (this could be added later)

So I think this covers all the use cases of sort_index(), but is slightly more general.

If you really want to sort a 1D DataArray by its own values, you would write da.sortby(da), but I agree that that will be rare.

What would the signature of sortby() be then? On our end we just want a more intuitive way to sort by dimension labels, so now I have sort_index(self, dims, ascending=True). sortby(), based on your description, seems like a separate method. Or any suggestion on how we can marry the two into something coherent?

@chunweiyuan I would skip inplace -- it's just not worth the complexity. It certainly does not make things any faster, so there is little gain from it. If you really want it, search for inplace in dataset.py and dataarray.py for examples.

When you assign to self, it creates a local variable: it doesn't override the object instance (Python doesn't support that).

I actually like the name sortby(), allowing any 1D variables as the argument (not just coordinates), as long as they have distinct dimensions. This works better for xarray than it does for pandas because we always have axis/dimension names.

A couple of things:

1.) Upon a little thinking I believe sort_values() doesn't make much sense, so I'm only working on sort_index()'. 2.) the way I handle theinplacekwarg is by ``` if inplace: self = self.isel(**{d: self.indexes[d].argsort() if ascending else self.indexes[d].argsort()[::-1] for d in dimensions}) else: return self.isel(**{d: self.indexes[d].argsort() if ascending else self.indexes[d].argsort()[::-1] for d in dimensions}) ``` But when I run ``` ds.sort_index(dims=['x', 'y'], inplace=True) ``` nothing changes. If I put apdb.set_trace()` right below the self = self*** I can evaluate self and see that it's what I want it to be. But somehow that assignment is not realized to the higher level. Any quick pointer?

If you pass in a list or array to isel, xarray copies the underling data like numpy does: http://xarray.pydata.org/en/stable/indexing.html#copies-vs-views

These approach do go do slightly different code paths, but they would have equivalent performance in most cases because the indexing cost will dominate over sorting. I would prefer using np.argsort() only because it's slightly more general, insofar as it doesn't rely on the index having unique labels. (If you have duplicate labels, reindex fails.) It also avoids needing to build the hash table for index based lookups, which has a small amount of overhead.

Also, as a side note np.sort and np.argsort are slightly faster than Python's sorted(), because can rely on homogeneous data dtypes.

On our end, we currently do the following when we need to sort by axis label (lat/lon in this case): da.reindex(indexers={'lat':sorted(da.coords['lat'].values), 'lon':sorted(da.coords['lon'].values)}) Upon first glance of the source code I think our approach goes down different code path than your .isel() approach. The most obvious difference, from a user's stand point, is probably that .reindex() returns a new object, whereas .isel() returns a view (typically). In Pandas, both sort_index() and sort_values() seem to return new objects.

We'd be happy to contribute to an xarray version of sort_index() and sort_values(). The first question is, which one would be the more robust and computationally efficient code path to take?