-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Should sel with slice objects care about underlying coordinate order? #1613
Comments
This was actually intentional, though you are certainly not the first person to complain about this. Xarray's behavior mirrors pandas. The justification for this logic is that Probably the cleanest solution possible would be a separate selection object for xarray that doesn't care about order, e.g., |
Given that EDIT: also then best to close this issue as wontfix |
To clarify: xarray behavior doesn't just mirror pandas here -- it actually directly uses pandas to figure out the mapping from labeled slices to integer slices. So I'm not opposed to changing this (I agree this trips people up often) but we should try to change it upstream if possible. |
Ah okay, that makes sense. I'm sure there's a related pandas issue (or many), will try to find that later. |
The only related issues I can find are:
They don't look identical though. Don't really have the time to dive into that further now. |
In case it helps anyone else, I ended up doing:
|
Consider using I also did a little more searching and found that this has come up in the xarray issue tracker before: #1465 Thinking about this a little more, I'm remembering the reasoning for pandas working this way:
|
Thanks, that's nicer, will do. And thanks for digging up the background/rationales.
This I don't think I agree with. Slicing by position and by label are semantically very different operations. (2) is correct, but irrelevant to label-based indexing. (3) yes, agree that's a mistake (4) indeed (5) I'd say that it's in practice less important, because users normally won't do Additionally: arguably monotonicity should not be required. When one uses labels, the semantics are clear without monotonicity. This doesn't have a position-based equivalent. |
Requiring monotonicity lets us guarantee that the result is a NumPy view rather than possibly being a copy. |
Sure, but if a users happens to have non-monotonic data it just requires her to then make that copy first anyway. Still a good thing overall for performance, but there'll be cases where it's just an extra thing to understand for the user without any performance gain. Anyway, the non-monotonic case is less relevant, because it's harder to run into in practice. The decreasing case however is easy - there is standard geo software (looking at you ArcGIS) that can write geoTiff's with monotonic decreasing indices. That's how I ran into this. Rewriting multi-GB source data that I didn't produce is not an option, so I'm left with the manual monotonicity checks and juggling label-based slices. |
This is very frequent for climate models also (lat arrays are frequently upside down). An xarray (or upstream) solution would be very useful indeed, but possibly there are other things to consider, like being "too magical". xarray makes a very good job at handling decreasing coordinates (i.e. when plotting), but soon or late users might need to know how they data is ordered, and indexing might be the right time to warn them about it when they do something wrong: usually an empty array is unlikely to be left unnoticed, so that the current behavior might help to find bugs in user code. |
Agreed, I don't think many pandas users would object to this. |
In order to maintain a list of currently relevant issues, we mark issues as stale after a period of inactivity If this issue remains relevant, please comment here or remove the |
This is probably worth fixing if possible in a straightforward way. I don't think anyone is well served by matching the behavior of Python list indexing here -- it's a strange edge that case that indexing a list like |
What would be a good solution?
|
How about: add one code line at the top of |
One complication with using
If we change the semantics of Alternatively, we could either do the dedicated indexing object like |
@shoyer, New syntax like your first suggestion, |
How about an informative warning when returning an empty set, as a minimal change? What's the path to an action item here? |
Another challenge with changing the meaning of I think the separate new API (e.g., |
Agreed, new rather than redefine existing meanings. |
xref #7099 about generic API for indexing objects. |
If I have a xarray DataArray, that has x-coordinates in descending order, and I try to crop the DataArray using
da.sel(x=slice(10,50))
, it gives me empty DataArray. If I useda.sel(x=slice(50,10))
, it finds everything it is supposed to find.I think it would be useful to be able to use this feature without knowing the exact order of the DataArray. Is this intentional?
The text was updated successfully, but these errors were encountered: