home / github

Menu
  • GraphQL API
  • Search all tables

issue_comments

Table actions
  • GraphQL API for issue_comments

7 rows where user = 19578931 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

issue 2

  • Change isinstance checks to duck Dask Array checks #4208 6
  • Support for duck Dask Arrays 1

user 1

  • rpmanser · 7 ✖

author_association 1

  • CONTRIBUTOR 7
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
685920915 https://github.com/pydata/xarray/pull/4221#issuecomment-685920915 https://api.github.com/repos/pydata/xarray/issues/4221 MDEyOklzc3VlQ29tbWVudDY4NTkyMDkxNQ== rpmanser 19578931 2020-09-02T18:32:45Z 2020-09-02T18:32:45Z CONTRIBUTOR

@dcherian No problem! Thanks for all your help!

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Change isinstance checks to duck Dask Array checks #4208 656163384
684052741 https://github.com/pydata/xarray/pull/4221#issuecomment-684052741 https://api.github.com/repos/pydata/xarray/issues/4221 MDEyOklzc3VlQ29tbWVudDY4NDA1Mjc0MQ== rpmanser 19578931 2020-08-31T21:31:51Z 2020-08-31T21:31:51Z CONTRIBUTOR

LGTM. The reason for requires_pint_0_15 would be nice but I don't feel strongly about it so it's up to you.

Thanks for being patient, @rpmanser.

No worries, thanks for the reviews!

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Change isinstance checks to duck Dask Array checks #4208 656163384
682306752 https://github.com/pydata/xarray/pull/4221#issuecomment-682306752 https://api.github.com/repos/pydata/xarray/issues/4221 MDEyOklzc3VlQ29tbWVudDY4MjMwNjc1Mg== rpmanser 19578931 2020-08-28T03:37:36Z 2020-08-28T03:37:36Z CONTRIBUTOR

I meant is_duck_array_or_ndarray, not is_duck_dask_array_or_ndarray. I don't suppose there's a way to fix that at this point?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Change isinstance checks to duck Dask Array checks #4208 656163384
681182191 https://github.com/pydata/xarray/pull/4221#issuecomment-681182191 https://api.github.com/repos/pydata/xarray/issues/4221 MDEyOklzc3VlQ29tbWVudDY4MTE4MjE5MQ== rpmanser 19578931 2020-08-26T23:54:18Z 2020-08-26T23:54:18Z CONTRIBUTOR

Tests are failing in test_formatting.py for test_diff_attrs_repr_with_array. This looks very similar to the previous problem that was occurring for duck array equality due to NEP18. Is a similar xfail appropriate here?

I'm not sure why FormattingBlack failed. It's claiming that 45 files would be reformatted. I used the pre-commit hook, which passed.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Change isinstance checks to duck Dask Array checks #4208 656163384
661226607 https://github.com/pydata/xarray/pull/4221#issuecomment-661226607 https://api.github.com/repos/pydata/xarray/issues/4221 MDEyOklzc3VlQ29tbWVudDY2MTIyNjYwNw== rpmanser 19578931 2020-07-20T17:35:31Z 2020-07-20T17:35:31Z CONTRIBUTOR

I have erred on the side of leaving the isinstance checks in place where @jthielen reviewed them. If it is decided that no further tests are needed, then this should be ready for review.

On another note, if it is appropriate to apply these changes:

Revisiting it I now realize that is_array_like is actually not a good name for that function: officially, "array_like" means that it can be casted using np.asarray. Maybe we should rename that to is_duck_array? But then we'd also need to check for __array_ufunc__ and __array_function__ because we also need those to properly wrap duck arrays.

As long as that doesn't break any of the current uses, I think that would be the best way forwards. This would require xarray to be on NumPy 1.16+ (in order to ensure hasattr(np.ndarray, "__array_function__"), but that's only 9 days away by NEP 29, so hopefully that isn't too much of an issue. Then, most of the checks for "can this be wrapped by xarray" should be able to become something like

python is_duck_array(x) and not isinstance(x, (DataArray, Variable))

with is_duck_dask_array only being needed for checking Dask Array-like support.

I can do that as well. If not, should a new issue be opened for this?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Change isinstance checks to duck Dask Array checks #4208 656163384
658946116 https://github.com/pydata/xarray/pull/4221#issuecomment-658946116 https://api.github.com/repos/pydata/xarray/issues/4221 MDEyOklzc3VlQ29tbWVudDY1ODk0NjExNg== rpmanser 19578931 2020-07-15T19:01:17Z 2020-07-15T19:01:17Z CONTRIBUTOR

I'm getting a bit lost within the testing suite while trying to figure out the implications of this change and what kinds of tests are necessary for it. Would a test module for this - Individually test all functions that include the newly implemented is_duck_dask_array()? - Test that a duck dask Array has attributes of a dask Array and redirects the appropriate methods to those defined by Variable, DataArray, and Dataset? - Be similar to test_dask.py? - All, some, or none of the above?

Any suggestions would be greatly appreciated.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Change isinstance checks to duck Dask Array checks #4208 656163384
656349279 https://github.com/pydata/xarray/issues/4208#issuecomment-656349279 https://api.github.com/repos/pydata/xarray/issues/4208 MDEyOklzc3VlQ29tbWVudDY1NjM0OTI3OQ== rpmanser 19578931 2020-07-09T21:01:07Z 2020-07-09T21:01:07Z CONTRIBUTOR

I can go ahead with putting together a PR for this. Before I do so, I'd like to clarify what is expected.

  • Implement the is_duck_dask_array() function
  • In that implementation, use dask.base.is_dask_collection() and the existing duck array check(s)
  • Replace isinstance(x, dask.array.Array) checks with the new is_dask_duck_array() function

I searched for existing duck array checks in xarray and nothing immediately obvious to me is showing up. It looks like a check for __array_function__ is inappropriate based on discussion in #3917. Could someone point out the proper duck array check?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Support for duck Dask Arrays 653430454

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