issue_comments

Table actions

6 rows where issue = 895713502 sorted by updated_at descending

✖
✖

✎ View and edit SQL

This data as json, CSV (advanced)

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

author_association 2

issue 1

  • Remove npcompat.moveaxis · 6 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
863419727 https://github.com/pydata/xarray/pull/5345#issuecomment-863419727 https://api.github.com/repos/pydata/xarray/issues/5345 MDEyOklzc3VlQ29tbWVudDg2MzQxOTcyNw== dcherian 2448579 2021-06-17T17:18:20Z 2021-06-17T17:18:20Z MEMBER

Thanks @Illviljan !

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove npcompat.moveaxis 895713502
860086402 https://github.com/pydata/xarray/pull/5345#issuecomment-860086402 https://api.github.com/repos/pydata/xarray/issues/5345 MDEyOklzc3VlQ29tbWVudDg2MDA4NjQwMg== max-sixty 5635139 2021-06-12T17:47:24Z 2021-06-12T17:47:24Z MEMBER

@shoyer if you get a moment, could you take a glance at this and see if it looks OK, or we should add more tests?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove npcompat.moveaxis 895713502
846113331 https://github.com/pydata/xarray/pull/5345#issuecomment-846113331 https://api.github.com/repos/pydata/xarray/issues/5345 MDEyOklzc3VlQ29tbWVudDg0NjExMzMzMQ== max-sixty 5635139 2021-05-21T17:14:34Z 2021-05-21T17:14:34Z MEMBER

Yes, pytest can be a bit confusing at first, but it's a really nice framework — da comes from https://github.com/pydata/xarray/blob/v0.18.2/xarray/tests/test_dataarray.py#L6458, which is parametrized with both a numpy-backed and a dask-backed array, so it creates two tests.

I agree this seems like it should be fine, and maybe I'm being a bit too cautious. @keewis could you give this a check given my track-record is blotchy now, and we can merge if you agree?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove npcompat.moveaxis 895713502
846078946 https://github.com/pydata/xarray/pull/5345#issuecomment-846078946 https://api.github.com/repos/pydata/xarray/issues/5345 MDEyOklzc3VlQ29tbWVudDg0NjA3ODk0Ng== Illviljan 14371165 2021-05-21T16:21:35Z 2021-05-21T16:21:35Z MEMBER

It's probably worth double-checking that the more complex Variable __getitem__ methods are also tested on Dask arrays. We could you an approach like #5350 if we're not confident.

I don't understand how this works, @max-sixty. You're adding the variable da to the method but where does da then later come from? It doesn't seem to be defined in this file?

The numpy implementation seems very similar to the npcompat version: https://github.com/numpy/numpy/blob/fb215c76967739268de71aa4bda55dd1b062bc2e/numpy/core/numeric.py#L1405

Maybe the npcompat code was copy/pasted just to avoid waiting on numpy releases?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove npcompat.moveaxis 895713502
845461228 https://github.com/pydata/xarray/pull/5345#issuecomment-845461228 https://api.github.com/repos/pydata/xarray/issues/5345 MDEyOklzc3VlQ29tbWVudDg0NTQ2MTIyOA== max-sixty 5635139 2021-05-20T20:39:25Z 2021-05-20T20:39:25Z MEMBER

It's probably worth double-checking that the more complex Variable __getitem__ methods are also tested on Dask arrays. We could you an approach like https://github.com/pydata/xarray/pull/5350 if we're not confident.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove npcompat.moveaxis 895713502
845403040 https://github.com/pydata/xarray/pull/5345#issuecomment-845403040 https://api.github.com/repos/pydata/xarray/issues/5345 MDEyOklzc3VlQ29tbWVudDg0NTQwMzA0MA== pep8speaks 24736507 2021-05-20T19:08:27Z 2021-05-20T20:31:33Z NONE

Hello @Illviljan! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2021-05-20 20:31:33 UTC
 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove npcompat.moveaxis 895713502

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