Skip to content

More Robust Implementation of slice #1465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jdossgollin opened this issue Jun 24, 2017 · 5 comments
Closed

More Robust Implementation of slice #1465

jdossgollin opened this issue Jun 24, 2017 · 5 comments

Comments

@jdossgollin
Copy link

Currently, only one of the two operations: ds.sel(lon = slice(X0, X1)) and ds.sel(lon = slice(X1, X0)) works; the other will give a data array (or dataset) with zero values. This can be a difficulty when working with different data sets which have different indexing methods.

It would be great to have slice implemented, at least in the context of sel, to check whether the index of ds is increasing or decreasing, and then take the min and max of X0 and X1 accordingly.

@fmaussion
Copy link
Member

While it seems a good idea at first (I've fallen into this as well), I'm not sure this would be a good idea. Look at pandas for example:

In [1]: import pandas as pd

In [2]: ts = pd.Series([1, 2, 3, 4,])

In [3]: ts[1:3]
Out[3]: 
1    2
2    3
dtype: int64

In [4]: ts[3:1]
Out[4]: Series([], dtype: int64)

I don't think one would like the last statement to return something else than an empty series.

@jdossgollin
Copy link
Author

Definitely a fair point, but would it be possible to add an argument to either slice or sel (probably slice) which defaults to False? For example, ds.sel(lon = slice(X0, X1, both_ways = True) would implement this behavior, but both_ways defaults to False.

@shoyer
Copy link
Member

shoyer commented Jun 25, 2017

I agree that this is somewhat surprising, but as @fmaussion points out this slicing behavior is consistent with pandas (and Python more generally, nothing that range(5)[3:1] is empty).

slice is a Python built-in, so we can't adjust it. What we could potentially do is add a new keyboard argument to .sel() (or add a new indexing method entirely).

@jdossgollin
Copy link
Author

Definitely want to leave it up to you guys but agree that putting it in slice probably doesn't make sense. Adding something to .sel() would be a cool feature in the spirit of projects like Aospy, which try to minimize the amount code re-writing necessary for carrying out simple analysis on different datasets which are often organized in subtly different ways. Of course, it's pretty easy for users to implement this ourselves if we really need. Thanks for considering!

@dcherian
Copy link
Contributor

Closing in favour of #1613 which has more discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants