issue_comments

Table actions

5 rows where issue = 272460887 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 2

issue 1

  • Make Indexer classes not inherit from tuple. · 5 ✖

author_association 1

id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
344136038 https://github.com/pydata/xarray/pull/1705#issuecomment-344136038 https://api.github.com/repos/pydata/xarray/issues/1705 MDEyOklzc3VlQ29tbWVudDM0NDEzNjAzOA== shoyer 1217238 2017-11-14T03:32:49Z 2017-11-14T03:32:49Z MEMBER

OK, merging. Hopefully we'll catch any issues in the next release candidate!

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Make Indexer classes not inherit from tuple. 272460887
344134665 https://github.com/pydata/xarray/pull/1705#issuecomment-344134665 https://api.github.com/repos/pydata/xarray/issues/1705 MDEyOklzc3VlQ29tbWVudDM0NDEzNDY2NQ== shoyer 1217238 2017-11-14T03:23:24Z 2017-11-14T03:23:24Z MEMBER

Actually, looking over @rabernat's custom backend code in xmitgcm, I think that should be OK. This would only be a problem if you pass custom duck arrays into xarray.Variable objects, rather than numpy or dask arrays.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Make Indexer classes not inherit from tuple. 272460887
344085798 https://github.com/pydata/xarray/pull/1705#issuecomment-344085798 https://api.github.com/repos/pydata/xarray/issues/1705 MDEyOklzc3VlQ29tbWVudDM0NDA4NTc5OA== shoyer 1217238 2017-11-13T22:49:39Z 2017-11-13T22:49:39Z MEMBER

I plan to merge this shortly and issue another release candidate unless any one has objections.

It does occur to me that this could break custom backends, since they will not longer be getting tuples as indexers. @rabernat any ideas for how to minimize that pain?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Make Indexer classes not inherit from tuple. 272460887
343236205 https://github.com/pydata/xarray/pull/1705#issuecomment-343236205 https://api.github.com/repos/pydata/xarray/issues/1705 MDEyOklzc3VlQ29tbWVudDM0MzIzNjIwNQ== shoyer 1217238 2017-11-09T17:51:40Z 2017-11-09T17:51:40Z MEMBER

There's been discussion about abstract classes on the NumPy mailing list. It would be nice to use something standard if possible: https://mail.python.org/pipermail/numpy-discussion/2017-November/thread.html#77312

On Thu, Nov 9, 2017 at 6:19 AM Keisuke Fujii notifications@github.com wrote:

This looks pretty clean and less error-prone.

For more cleanliness, I'm wondering if we could more clearly distinguish between raw array-wrappers (such as NumpyIndexingAdapter) and wrappers of array-wrapper (such as MemoryCachedArray). But as a whole, I like this idea.

Regarding the more array-type support in the future (as suggested in comment https://github.com/pydata/xarray/issues/1617#issuecomment-340290288), is there something to prepare in this PR? I guess there are some typical indexing types, such as Fortran-type and Numpy-type. Can we have some abstract classes (maybe too early)?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/1705#issuecomment-343167809, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKS1tRYI6F0WTmR9ozdBdvy0g6Yi-Ipks5s0wnagaJpZM4QXhhG . 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Make Indexer classes not inherit from tuple. 272460887
343167809 https://github.com/pydata/xarray/pull/1705#issuecomment-343167809 https://api.github.com/repos/pydata/xarray/issues/1705 MDEyOklzc3VlQ29tbWVudDM0MzE2NzgwOQ== fujiisoup 6815844 2017-11-09T14:19:04Z 2017-11-09T14:19:04Z MEMBER

This looks pretty clean and less error-prone.

For more cleanliness, I'm wondering if we could more clearly distinguish between raw array-wrappers (such as NumpyIndexingAdapter) and wrappers of array-wrapper (such as MemoryCachedArray). But as a whole, I like this idea.

Regarding the more array-type support in the future (as suggested in comment), is there something to prepare in this PR? I guess there are some typical indexing types, such as Fortran-type and Numpy-type. Can we have some abstract classes (maybe too early)?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Make Indexer classes not inherit from tuple. 272460887

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