issue_comments

Table actions

11 rows where issue = 819911891 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 2

author_association 2

issue 1

  • Adds Dataset.query() method, analogous to pandas DataFrame.query() · 11 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
800504527 https://github.com/pydata/xarray/pull/4984#issuecomment-800504527 https://api.github.com/repos/pydata/xarray/issues/4984 MDEyOklzc3VlQ29tbWVudDgwMDUwNDUyNw== alimanfoo 703554 2021-03-16T18:28:09Z 2021-03-16T18:28:09Z CONTRIBUTOR

Yay, first xarray PR :partying_face: 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Adds Dataset.query() method, analogous to pandas DataFrame.query() 819911891
800461714 https://github.com/pydata/xarray/pull/4984#issuecomment-800461714 https://api.github.com/repos/pydata/xarray/issues/4984 MDEyOklzc3VlQ29tbWVudDgwMDQ2MTcxNA== max-sixty 5635139 2021-03-16T17:28:11Z 2021-03-16T17:28:11Z MEMBER

Great, merging!

Seconded re the docs! 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Adds Dataset.query() method, analogous to pandas DataFrame.query() 819911891
800317378 https://github.com/pydata/xarray/pull/4984#issuecomment-800317378 https://api.github.com/repos/pydata/xarray/issues/4984 MDEyOklzc3VlQ29tbWVudDgwMDMxNzM3OA== alimanfoo 703554 2021-03-16T14:40:45Z 2021-03-16T14:40:45Z CONTRIBUTOR

Could we add a very small test for the DataArray? Given the coverage on Dataset, it should mostly just test that the method works.

No problem, some DataArray tests are there.

Any thoughts from others before we merge?

Good to go from my side.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Adds Dataset.query() method, analogous to pandas DataFrame.query() 819911891
800308560 https://github.com/pydata/xarray/pull/4984#issuecomment-800308560 https://api.github.com/repos/pydata/xarray/issues/4984 MDEyOklzc3VlQ29tbWVudDgwMDMwODU2MA== max-sixty 5635139 2021-03-16T14:30:40Z 2021-03-16T14:30:40Z MEMBER

Excellent!

Could we add a very small test for the DataArray? Given the coverage on Dataset, it should mostly just test that the method works.

Any thoughts from others before we merge?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Adds Dataset.query() method, analogous to pandas DataFrame.query() 819911891
800176868 https://github.com/pydata/xarray/pull/4984#issuecomment-800176868 https://api.github.com/repos/pydata/xarray/issues/4984 MDEyOklzc3VlQ29tbWVudDgwMDE3Njg2OA== alimanfoo 703554 2021-03-16T11:24:42Z 2021-03-16T11:24:42Z CONTRIBUTOR

Hi @max-sixty,

It looks like we need a requires_numexpr decorator on the tests — would you be OK to add that?

Sure, done.

Could we add a simple method to DataArray which converts to a Dataset, calls the functions, and converts back too? (there are lots of examples already of this, let me know any issues)

Done.

And we should add the methods to api.rst, and a whatsnew entry if possible.

Done.

Let me know if there's anything else. Looking forward to using this :smile: 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Adds Dataset.query() method, analogous to pandas DataFrame.query() 819911891
799076455 https://github.com/pydata/xarray/pull/4984#issuecomment-799076455 https://api.github.com/repos/pydata/xarray/issues/4984 MDEyOklzc3VlQ29tbWVudDc5OTA3NjQ1NQ== max-sixty 5635139 2021-03-15T04:13:46Z 2021-03-15T04:13:46Z MEMBER

Great re the dimensions!

I reviewed the tests more fully, they look great.

It looks like we need a requires_numexpr decorator on the tests — would you be OK to add that?

Could we add a simple method to DataArray which converts to a Dataset, calls the functions, and converts back too? (there are lots of examples already of this, let me know any issues)

And we should add the methods to api.rst, and a whatsnew entry if possible.

Does anyone have any other thoughts? I think the API is very reasonable. I could imagine a more sophisticated API that could take a single query, rather than a dict of them by dimension — currently it's da.query(x="x>3", y="y>4"), but that would require more work and decisions, even if it were preferable.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Adds Dataset.query() method, analogous to pandas DataFrame.query() 819911891
798993998 https://github.com/pydata/xarray/pull/4984#issuecomment-798993998 https://api.github.com/repos/pydata/xarray/issues/4984 MDEyOklzc3VlQ29tbWVudDc5ODk5Mzk5OA== alimanfoo 703554 2021-03-14T22:44:49Z 2021-03-14T22:44:49Z CONTRIBUTOR

Currently the test runs over an array of two dimensions — x & y. Would pd.query work if there were also a z dimension?

No worries, yes any number of dimensions can be queried. I've added tests showing three dimensions can be queried.

As an aside, in writing these tests I came upon a probable upstream bug in pandas, reported as https://github.com/pandas-dev/pandas/issues/40436. I don't think this affects this PR though, and has low impact as only the "python" query parser is affected, and most people will use the default "pandas" query parser. 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Adds Dataset.query() method, analogous to pandas DataFrame.query() 819911891
797842181 https://github.com/pydata/xarray/pull/4984#issuecomment-797842181 https://api.github.com/repos/pydata/xarray/issues/4984 MDEyOklzc3VlQ29tbWVudDc5Nzg0MjE4MQ== max-sixty 5635139 2021-03-13T01:27:32Z 2021-03-13T01:27:32Z MEMBER

Hi @max-sixty, no problem. Re this...

Does the pd.eval work with more than two dimensions?

...not quite sure what you mean, could you elaborate?

For sure — forgive me if I wasn't clear.

Currently the test runs over an array of two dimensions — x & y. Would pd.query work if there were also a z dimension?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Adds Dataset.query() method, analogous to pandas DataFrame.query() 819911891
797668635 https://github.com/pydata/xarray/pull/4984#issuecomment-797668635 https://api.github.com/repos/pydata/xarray/issues/4984 MDEyOklzc3VlQ29tbWVudDc5NzY2ODYzNQ== alimanfoo 703554 2021-03-12T18:16:15Z 2021-03-12T18:16:15Z CONTRIBUTOR

Just to mention I've added tests to verify this works with variables backed by dask arrays. Also added explicit tests of different eval engine and query parser options. And added a docstring.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Adds Dataset.query() method, analogous to pandas DataFrame.query() 819911891
797636489 https://github.com/pydata/xarray/pull/4984#issuecomment-797636489 https://api.github.com/repos/pydata/xarray/issues/4984 MDEyOklzc3VlQ29tbWVudDc5NzYzNjQ4OQ== alimanfoo 703554 2021-03-12T17:21:29Z 2021-03-12T17:21:29Z CONTRIBUTOR

Hi @max-sixty, no problem. Re this...

Does the pd.eval work with more than two dimensions?

...not quite sure what you mean, could you elaborate?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Adds Dataset.query() method, analogous to pandas DataFrame.query() 819911891
788828644 https://github.com/pydata/xarray/pull/4984#issuecomment-788828644 https://api.github.com/repos/pydata/xarray/issues/4984 MDEyOklzc3VlQ29tbWVudDc4ODgyODY0NA== alimanfoo 703554 2021-03-02T11:10:20Z 2021-03-02T11:10:20Z CONTRIBUTOR

Hi folks, thought I'd put up a proof of concept PR here for further discussion. Any advice/suggestions about if/how to take this forward would be very welcome.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Adds Dataset.query() method, analogous to pandas DataFrame.query() 819911891

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