home / github

Menu
  • Search all tables
  • GraphQL API

issue_comments

Table actions
  • GraphQL API for issue_comments

21 rows where issue = 263403430 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

Suggested facets: reactions, created_at (date), updated_at (date)

user 6

  • shoyer 8
  • rgommers 6
  • brianmapes 4
  • dcherian 1
  • fmaussion 1
  • stale[bot] 1

author_association 2

  • NONE 11
  • MEMBER 10

issue 1

  • Should sel with slice objects care about underlying coordinate order? · 21 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1062281975 https://github.com/pydata/xarray/issues/1613#issuecomment-1062281975 https://api.github.com/repos/pydata/xarray/issues/1613 IC_kwDOAMm_X84_USL3 brianmapes 2086210 2022-03-08T22:26:07Z 2022-03-08T22:26:07Z NONE

Agreed, new rather than redefine existing meanings. Who has the chops and the cred to implement it? Not quite me...

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
1062211273 https://github.com/pydata/xarray/issues/1613#issuecomment-1062211273 https://api.github.com/repos/pydata/xarray/issues/1613 IC_kwDOAMm_X84_UA7J shoyer 1217238 2022-03-08T21:09:05Z 2022-03-08T21:09:05Z MEMBER

Another challenge with changing the meaning of slice is handling partial slices, e.g., what does slice(500, None) mean? With a monotonic decreasing index, that would select values below 500, but ignoring underlying coordinate order it would presumably mean selecting values above 500.

I think the separate new API (e.g., xarray.Between or .sel_between()) is probably a better idea.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
1062142845 https://github.com/pydata/xarray/issues/1613#issuecomment-1062142845 https://api.github.com/repos/pydata/xarray/issues/1613 IC_kwDOAMm_X84_TwN9 brianmapes 2086210 2022-03-08T19:49:33Z 2022-03-08T19:49:33Z NONE

How about an informative warning when returning an empty set, as a minimal change? What's the path to an action item here?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
1058446923 https://github.com/pydata/xarray/issues/1613#issuecomment-1058446923 https://api.github.com/repos/pydata/xarray/issues/1613 IC_kwDOAMm_X84_Fp5L brianmapes 2086210 2022-03-03T20:19:56Z 2022-03-03T20:19:56Z NONE

@shoyer, selection over non-monotonic indexes, merely based on matching bounds is a case I never considered... probably a rare one, which I for one would happily sacrifice.

New syntax like your first suggestion, .Between instead of .slice if I understand correctly, could be appealing too: The word 'slice' has never seemed terribly intuitive to this user, and 'between' is high in an alphabetical list of tips/suggestions a good IDE might bring up with the tab key or whatever.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
1058341139 https://github.com/pydata/xarray/issues/1613#issuecomment-1058341139 https://api.github.com/repos/pydata/xarray/issues/1613 IC_kwDOAMm_X84_FQET brianmapes 2086210 2022-03-03T18:13:31Z 2022-03-03T20:03:15Z NONE

How about: add one code line at the top of slice that properly orders the exactly two arguments (in other words, reverses the order if necessary)? I haven't studied the code at all, this may be naive. This may the same as 1. above (make sel ignore order). Ideally the default would be silently improved, rather than creating something optional that users would have to make a new habit of, like an additional kwarg or new syntax.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
1058366320 https://github.com/pydata/xarray/issues/1613#issuecomment-1058366320 https://api.github.com/repos/pydata/xarray/issues/1613 IC_kwDOAMm_X84_FWNw shoyer 1217238 2022-03-03T18:39:59Z 2022-03-03T18:39:59Z MEMBER

One complication with using sel() with slice objects is that you can do selection over non-monotonic indexes, merely based on matching bounds: ```

data = xarray.DataArray([1, 2, 3, 4, 5], dims=['x'], coords=[[5, 1, 4, 3, 2]]) data <xarray.DataArray (x: 5)> array([1, 2, 3, 4, 5]) Coordinates: * x (x) int64 5 1 4 3 2 data.sel(x=slice(1, 3))) <xarray.DataArray (x: 3)> array([2, 3, 4]) Coordinates: * x (x) int64 1 4 3 ```

If we change the semantics of slice in sel() to do filtering rather than be concerned about order (which does seem much less useful), we should probably deprecate the handling of non-monotonic ascending or descending indexes.

Alternatively, we could either do the dedicated indexing object like xarray.Between(lower, upper) or have a dedicated method for selecting between values, e.g., perhaps data.sel_between(x=(1, 3)) or data.sel_bounds(x=(1, 3)).

{
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 1
}
  Should sel with slice objects care about underlying coordinate order? 263403430
1058335738 https://github.com/pydata/xarray/issues/1613#issuecomment-1058335738 https://api.github.com/repos/pydata/xarray/issues/1613 IC_kwDOAMm_X84_FOv6 dcherian 2448579 2022-03-03T18:07:16Z 2022-03-03T18:07:16Z MEMBER

What would be a good solution? 1. Make sel implicitly ignore order for slice objects. 2. Add a option to sel for explicitly ignoring slice order. 1. add a new kwarg 2. Add a new xr.Between object

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
1058293194 https://github.com/pydata/xarray/issues/1613#issuecomment-1058293194 https://api.github.com/repos/pydata/xarray/issues/1613 IC_kwDOAMm_X84_FEXK shoyer 1217238 2022-03-03T17:23:09Z 2022-03-03T17:23:09Z MEMBER

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 x[5:0] returns an empty list.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
678676112 https://github.com/pydata/xarray/issues/1613#issuecomment-678676112 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDY3ODY3NjExMg== stale[bot] 26384082 2020-08-22T18:34:59Z 2020-08-22T18:34:59Z NONE

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 stale label; otherwise it will be marked as closed automatically

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
422219032 https://github.com/pydata/xarray/issues/1613#issuecomment-422219032 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDQyMjIxOTAzMg== shoyer 1217238 2018-09-18T01:02:30Z 2018-09-18T01:02:30Z MEMBER

Making slice(high, low) return the same as slice(low, high) on a monotonic increasing index seems reasonable.

Agreed, I don't think many pandas users would object to this.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
422098003 https://github.com/pydata/xarray/issues/1613#issuecomment-422098003 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDQyMjA5ODAwMw== fmaussion 10050469 2018-09-17T17:17:18Z 2018-09-17T17:17:18Z MEMBER

there is standard geo software (looking at you ArcGIS) that can write geoTiff's with monotonic decreasing indices

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.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
422090593 https://github.com/pydata/xarray/issues/1613#issuecomment-422090593 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDQyMjA5MDU5Mw== rgommers 98330 2018-09-17T16:53:59Z 2018-09-17T16:53:59Z NONE

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.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
422085754 https://github.com/pydata/xarray/issues/1613#issuecomment-422085754 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDQyMjA4NTc1NA== shoyer 1217238 2018-09-17T16:39:39Z 2018-09-17T16:39:39Z MEMBER

Additionally: arguably monotonicity should not be required. When one uses labels, the semantics are clear without monotonicity.

Requiring monotonicity lets us guarantee that the result is a NumPy view rather than possibly being a copy.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
422083146 https://github.com/pydata/xarray/issues/1613#issuecomment-422083146 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDQyMjA4MzE0Ng== rgommers 98330 2018-09-17T16:31:51Z 2018-09-17T16:31:51Z NONE

Consider using index.is_monotonic_ascending and index.is_monotonic_descending instead of subtracting the first few values -- those are exactly the checks that pandas uses.

Thanks, that's nicer, will do. And thanks for digging up the background/rationales.

(1)Integer slicing by position and by label with positive integers should work the same for an index with values given by range(N).

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 slice(high, low), but for consistency I agree that there should be symmetry in behaviour. Making slice(high, low) return the same as slice(low, high) on a monotonic increasing index seems reasonable.

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.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
422076318 https://github.com/pydata/xarray/issues/1613#issuecomment-422076318 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDQyMjA3NjMxOA== shoyer 1217238 2018-09-17T16:09:54Z 2018-09-17T16:09:54Z MEMBER

Consider using index.is_monotonic_ascending and index.is_monotonic_descending instead of subtracting the first few values -- those are exactly the checks that pandas uses.

I also did a little more searching and found that this has come up in the xarray issue tracker before: https://github.com/pydata/xarray/issues/1465


Thinking about this a little more, I'm remembering the reasoning for pandas working this way:

  1. Integer slicing by position and by label with positive integers should work the same for an index with values given by range(N).
  2. Positional indexing in Python x[a:b] returns all values between a and b in order. Indexing like x[high:low] with high > low returns an empty list.
  3. Label based indexing in pandas has df.loc[df.index[i] : df.index[j]] equivalent to df.iloc[i : j+1] (arguably this is a mistake; e.g., https://github.com/pydata/xarray/issues/1492 and other discussion that I can't find right now :) ).
  4. Indexing a monotonic decreasing index therefore should work for decreasing labels.
  5. If slicing a monotonic increasing index only works for increasing but not decreasing labels, then it would be weird if indexing a monotonic decreasing index works for both increasing and decreasing labels.
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
421676579 https://github.com/pydata/xarray/issues/1613#issuecomment-421676579 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDQyMTY3NjU3OQ== rgommers 98330 2018-09-16T02:39:50Z 2018-09-16T02:39:50Z NONE

In case it helps anyone else, I ended up doing: ``` # Note that xarray is fiddly with indexing - if x or y values are ordered # high to low, then the slice bounds need to be reversed. So check that x_ordered_low2high = data.x.values[-1] - data.x.values[0] > 0 y_ordered_low2high = data.y.values[-1] - data.y.values[0] > 0

if x_ordered_low2high:
    x_index = slice(lower_lon, upper_lon)
else:
    x_index = slice(upper_lon, lower_lon)

if y_ordered_low2high:
    y_index = slice(lower_lat, upper_lat)
else:
    y_index = slice(upper_lat, lower_lat)

subset = data.sel(x=x_index, y=y_index)

```

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
421676344 https://github.com/pydata/xarray/issues/1613#issuecomment-421676344 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDQyMTY3NjM0NA== rgommers 98330 2018-09-16T02:37:40Z 2018-09-16T02:37:40Z NONE

The only related issues I can find are:

  • https://github.com/pandas-dev/pandas/issues/14316
  • https://github.com/pandas-dev/pandas/issues/7860

They don't look identical though. Don't really have the time to dive into that further now.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
420296389 https://github.com/pydata/xarray/issues/1613#issuecomment-420296389 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDQyMDI5NjM4OQ== rgommers 98330 2018-09-11T14:35:19Z 2018-09-11T14:35:19Z NONE

Ah okay, that makes sense. I'm sure there's a related pandas issue (or many), will try to find that later.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
420107363 https://github.com/pydata/xarray/issues/1613#issuecomment-420107363 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDQyMDEwNzM2Mw== shoyer 1217238 2018-09-11T00:43:14Z 2018-09-11T00:43:14Z MEMBER

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.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
420084948 https://github.com/pydata/xarray/issues/1613#issuecomment-420084948 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDQyMDA4NDk0OA== rgommers 98330 2018-09-10T22:40:15Z 2018-09-10T22:41:58Z NONE

The justification for this logic is that sel/loc would work like normal indexing [] on a Python list, and if you supply slice limits in the opposite order on a Python list, you get an empty result.

Given that sel is label/value-based, I don't see how the analogy with positional list-based indexing applies. This behavior looks like a functional bug. If you have decided to keep it as is, I'd suggest at the very least to add a warning if an empty dataset is returned because values in an attribute are ordered high...low.

EDIT: also then best to close this issue as wontfix

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430
335282551 https://github.com/pydata/xarray/issues/1613#issuecomment-335282551 https://api.github.com/repos/pydata/xarray/issues/1613 MDEyOklzc3VlQ29tbWVudDMzNTI4MjU1MQ== shoyer 1217238 2017-10-09T20:48:46Z 2017-10-09T20:48:46Z MEMBER

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 sel/loc would work like normal indexing [] on a Python list, and if you supply slice limits in the opposite order on a Python list, you get an empty result.

Probably the cleanest solution possible would be a separate selection object for xarray that doesn't care about order, e.g., da.sel(x=xarray.Between(10, 50)).

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Should sel with slice objects care about underlying coordinate order? 263403430

Advanced export

JSON shape: default, array, newline-delimited, object

CSV options:

CREATE TABLE [issue_comments] (
   [html_url] TEXT,
   [issue_url] TEXT,
   [id] INTEGER PRIMARY KEY,
   [node_id] TEXT,
   [user] INTEGER REFERENCES [users]([id]),
   [created_at] TEXT,
   [updated_at] TEXT,
   [author_association] TEXT,
   [body] TEXT,
   [reactions] TEXT,
   [performed_via_github_app] TEXT,
   [issue] INTEGER REFERENCES [issues]([id])
);
CREATE INDEX [idx_issue_comments_issue]
    ON [issue_comments] ([issue]);
CREATE INDEX [idx_issue_comments_user]
    ON [issue_comments] ([user]);
Powered by Datasette · Queries took 17.568ms · About: xarray-datasette