issue_comments

Table actions

9 rows where author_association = "MEMBER" and issue = 342793201 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

issue 1

  • WIP Add a CFTimeIndex-enabled xr.cftime_range function · 9 ✖

author_association 1

  • MEMBER · 9 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
422945794 https://github.com/pydata/xarray/pull/2301#issuecomment-422945794 https://api.github.com/repos/pydata/xarray/issues/2301 MDEyOklzc3VlQ29tbWVudDQyMjk0NTc5NA== shoyer 1217238 2018-09-19T20:24:51Z 2018-09-19T20:24:51Z MEMBER

thanks @spencerkclark !

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   WIP Add a CFTimeIndex-enabled xr.cftime_range function 342793201
421467827 https://github.com/pydata/xarray/pull/2301#issuecomment-421467827 https://api.github.com/repos/pydata/xarray/issues/2301 MDEyOklzc3VlQ29tbWVudDQyMTQ2NzgyNw== spencerkclark 6628425 2018-09-14T19:56:14Z 2018-09-14T19:56:14Z MEMBER

Many thanks for the initial review @shoyer. I think I got to everything so far. I agree restricting cftime_range to only return CFTimeIndexes makes things much simpler to reason about.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   WIP Add a CFTimeIndex-enabled xr.cftime_range function 342793201
420363884 https://github.com/pydata/xarray/pull/2301#issuecomment-420363884 https://api.github.com/repos/pydata/xarray/issues/2301 MDEyOklzc3VlQ29tbWVudDQyMDM2Mzg4NA== jhamman 2443309 2018-09-11T17:57:44Z 2018-09-11T17:57:44Z MEMBER

I took a quick look at this today. It looks quite complete and I'm eager to get this in to master. I'm personally having trouble breaking off enough time to give it a full review. If anyone else in @pydata/xarray has some time to give this a close look, I'm sure @spencerkclark would really appreciate it. 

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   WIP Add a CFTimeIndex-enabled xr.cftime_range function 342793201
417858978 https://github.com/pydata/xarray/pull/2301#issuecomment-417858978 https://api.github.com/repos/pydata/xarray/issues/2301 MDEyOklzc3VlQ29tbWVudDQxNzg1ODk3OA== spencerkclark 6628425 2018-09-01T13:15:34Z 2018-09-01T13:15:34Z MEMBER

Awesome, thanks @shoyer.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   WIP Add a CFTimeIndex-enabled xr.cftime_range function 342793201
417717093 https://github.com/pydata/xarray/pull/2301#issuecomment-417717093 https://api.github.com/repos/pydata/xarray/issues/2301 MDEyOklzc3VlQ29tbWVudDQxNzcxNzA5Mw== shoyer 1217238 2018-08-31T16:25:29Z 2018-08-31T16:25:29Z MEMBER

if you merge in master again, the test suite should be passing after https://github.com/pydata/xarray/pull/2393

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   WIP Add a CFTimeIndex-enabled xr.cftime_range function 342793201
417436579 https://github.com/pydata/xarray/pull/2301#issuecomment-417436579 https://api.github.com/repos/pydata/xarray/issues/2301 MDEyOklzc3VlQ29tbWVudDQxNzQzNjU3OQ== spencerkclark 6628425 2018-08-30T19:20:11Z 2018-08-30T19:20:11Z MEMBER

Thanks @jhamman! This should be synced up and ready for review. The test failure under the dask-dev build doesn't appear to be related.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   WIP Add a CFTimeIndex-enabled xr.cftime_range function 342793201
417135916 https://github.com/pydata/xarray/pull/2301#issuecomment-417135916 https://api.github.com/repos/pydata/xarray/issues/2301 MDEyOklzc3VlQ29tbWVudDQxNzEzNTkxNg== jhamman 2443309 2018-08-29T23:07:18Z 2018-08-29T23:07:18Z MEMBER

@spencerkclark - sync this branch with master and we'll get this all wrapped up. 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   WIP Add a CFTimeIndex-enabled xr.cftime_range function 342793201
407087972 https://github.com/pydata/xarray/pull/2301#issuecomment-407087972 https://api.github.com/repos/pydata/xarray/issues/2301 MDEyOklzc3VlQ29tbWVudDQwNzA4Nzk3Mg== shoyer 1217238 2018-07-23T14:58:22Z 2018-07-23T14:58:22Z MEMBER

I haven't had the chance to look in detail at this yet, but one small point I would suggest is renaming it from xarray.date_time to xarray.cftime_range. This makes it more obvious that the function is really for users of cftime.

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   WIP Add a CFTimeIndex-enabled xr.cftime_range function 342793201
406894349 https://github.com/pydata/xarray/pull/2301#issuecomment-406894349 https://api.github.com/repos/pydata/xarray/issues/2301 MDEyOklzc3VlQ29tbWVudDQwNjg5NDM0OQ== spencerkclark 6628425 2018-07-22T20:28:09Z 2018-07-22T20:28:09Z MEMBER

@jhamman @shoyer when you get a chance, I think this is ready for review.

I did a few more things since first pushing this PR: - Previously CFTimeIndex would raise an error if it contained an empty array; I'm not really sure what the use case would be, but to be consistent with the way DatetimeIndex behaves, I made modifications to allow for this (in some cases date_range returns an empty index). I also added the ability to give a CFTimeIndex a name. - The repr of CFTimeIndex previously provided no information regarding the calendar type of the index; I found this somewhat inconvenient, so I added a custom __unicode__ method to CFTimeIndex, which pandas.Index uses to build its repr. I think the result looks decent (I updated the examples above), but the implementation is a bit crude (relying on a number of private API functions of the base Index class). Maybe there is a better way or maybe we should put this off until later? - I added arguments to the constructor for CFTimeIndex that makes it more analogous to the constructor for a DatetimeIndex, allowing one to either pass dates directly, or create dates using arguments one can pass to date_range (inspired by https://github.com/pydata/xarray/issues/2140#issuecomment-389590577).

I hope the general approach seems reasonable, though if you had something else in mind for how to implement this originally, I'd be open to changing things. I tried to keep things as basic as I could (the pandas date_range function is fairly involved!). 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   WIP Add a CFTimeIndex-enabled xr.cftime_range function 342793201

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]);