home / github

Menu
  • Search all tables
  • GraphQL API

issue_comments

Table actions
  • GraphQL API for issue_comments

5 rows where author_association = "MEMBER", issue = 264321376 and user = 1217238 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 1

  • shoyer · 5 ✖

issue 1

  • Undesired decoding to timedelta64 (was: units of "seconds" translated to time coordinate) · 5 ✖

author_association 1

  • MEMBER · 5 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1114173984 https://github.com/pydata/xarray/issues/1621#issuecomment-1114173984 https://api.github.com/repos/pydata/xarray/issues/1621 IC_kwDOAMm_X85CaPIg shoyer 1217238 2022-05-01T08:49:40Z 2022-05-01T08:49:40Z MEMBER

Still relevant!

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Undesired decoding to timedelta64 (was: units of "seconds" translated to time coordinate) 264321376
614893356 https://github.com/pydata/xarray/issues/1621#issuecomment-614893356 https://api.github.com/repos/pydata/xarray/issues/1621 MDEyOklzc3VlQ29tbWVudDYxNDg5MzM1Ng== shoyer 1217238 2020-04-16T21:01:01Z 2020-04-16T21:01:01Z MEMBER

I still think the ideal path forward is https://github.com/pydata/xarray/issues/1621#issuecomment-339116478, but clearly nobody was excited about taking on this effort :).

I do still think we probably should retain a way to serialize/unserialize timedelta64 data before we switch the default behavior, rather than breaking existing users without any recourse.

That said, we certainly could add an optional flag for disabling decoding to timedelta64 (e.g., decode_timedelta=False in open_dataset/decode_cf) now, without changing anything else in xarray. The default flag switch could be saved until later, when the new timedelta64 serialization (steps 1 and 2 from above) works.

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Undesired decoding to timedelta64 (was: units of "seconds" translated to time coordinate) 264321376
339116478 https://github.com/pydata/xarray/issues/1621#issuecomment-339116478 https://api.github.com/repos/pydata/xarray/issues/1621 MDEyOklzc3VlQ29tbWVudDMzOTExNjQ3OA== shoyer 1217238 2017-10-24T20:12:32Z 2017-10-24T20:12:32Z MEMBER

OK, sounds like there is consensus on removing this. I would still like to there to be an option for doing this sort of decoding, because I'm sure somebody finds this useful (at least I did, back when I wrote it!).

In particular, it would be nice to have some way to round-trip the timedelta64 dtype. A simple way to do this would be to recognize the attribute dtype='timedelta64[ns] (as an xarray-specific convention) and use that for decoding/encoding timedelta64 dtypes.

My suggested path forward: 1. Add decoding support for recognizing dtype='timedelta64[ns]' and decoding it into the NumPy dtype. We have some very similar examples already (e.g., for dtype=bool), so this should not be hard. 2. Write all timedelta64 dtype data in netCDF files by saving the dtype attribute instead of units. 3. Issue a FutureWarning about what's going on that is triggered whenever unit='time_unit' is detected. 4. In the next major release of xarray, stop decoding time units.

Anyone interested in taking this on? All the logic can be found in xarray/conventions.py.

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Undesired decoding to timedelta64 (was: units of "seconds" translated to time coordinate) 264321376
338247940 https://github.com/pydata/xarray/issues/1621#issuecomment-338247940 https://api.github.com/repos/pydata/xarray/issues/1621 MDEyOklzc3VlQ29tbWVudDMzODI0Nzk0MA== shoyer 1217238 2017-10-20T15:57:14Z 2017-10-20T15:57:14Z MEMBER

I understand the potential issue here, but I think Xarray should follow CF conventions for time, and only treat variables as time coordinates if they have valid CF time units (<time unit> since <date>).

Rich, I know you've been involved in CF conventions and standard names, but I don't think the CF conventions on time apply directly here. These are "time difference" units, which are a distinct type of quantity. And assuredly the period of a wave is a type of "time difference" as well -- it's just not one that is useful to decode into an array with dtype np.timedelta64. Really this is a limitation of the non-ideal support for units in the NumPy ecosystem.

Is there some other sort of metadata we could use to make this distinction of "physical" vs. "human" time differences? Now is a good time to make changes, since we are on the verge of making a major release (v0.10).

Throwing out some ideas, none of which I particularly like (but to be clear, I don't like the status quo either): 1. We could unilaterally stop automatic decoding into timedelta64. (We would need to add a separate helper function that could be called to do these conversions afterwards.) 2. We could add look-up tables that recognize standard_name attributes, either a white-list or black-list for timedelta64 compatible variables. (This would be a first for xarray, and is not something I'm particularly looking forward to maintaining.) 3. We could decode coordinates and data variables differently, only converting coordinates into timedelta64. (This is not entirely ideal either, since it's easy to switch between the two in xarray's data model.)

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Undesired decoding to timedelta64 (was: units of "seconds" translated to time coordinate) 264321376
335608148 https://github.com/pydata/xarray/issues/1621#issuecomment-335608148 https://api.github.com/repos/pydata/xarray/issues/1621 MDEyOklzc3VlQ29tbWVudDMzNTYwODE0OA== shoyer 1217238 2017-10-10T21:06:30Z 2017-10-10T21:06:30Z MEMBER

Thanks for filing an issue -- I usually follow the xarray tag on StackOverflow but missed this one.

I wrote an answer on StackOverflow to explain why this works this way and how to work around it, but as I say in my answer although this is intended behavior I'm open to ideas for improvement. You are not the first person to complain about this, specifically for handling of wave periods. See https://github.com/pydata/xarray/issues/843 and links therein for prior discussion (CC @ocefpaf).

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Undesired decoding to timedelta64 (was: units of "seconds" translated to time coordinate) 264321376

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 188.236ms · About: xarray-datasette