issue_comments

Table actions

3 rows where issue = 1090229430 and user = 5635139 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 1

  • max-sixty · 3 ✖

issue 1

  • bool(ds) should raise a "the truth value of a Dataset is ambiguous" error · 3 ✖

author_association 1

id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1004669545 https://github.com/pydata/xarray/issues/6124#issuecomment-1004669545 https://api.github.com/repos/pydata/xarray/issues/6124 IC_kwDOAMm_X8474gpp max-sixty 5635139 2022-01-04T09:58:51Z 2022-01-04T09:58:51Z MEMBER

TBC, I am fine merging! I would lightly vote against it, but weigh my vote far below @shoyer 's.

And more broadly let's not wait for everyone to agree on everything!

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   bool(ds) should raise a "the truth value of a Dataset is ambiguous" error 1090229430
1003482768 https://github.com/pydata/xarray/issues/6124#issuecomment-1003482768 https://api.github.com/repos/pydata/xarray/issues/6124 IC_kwDOAMm_X847z-6Q max-sixty 5635139 2022-01-01T01:50:07Z 2022-01-01T01:50:37Z MEMBER

Would this mean that Dataset doesn't inherit from Mapping (in the end-state, after deprecation)? Or that we inherit from Mapping but override bool to raise an error?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   bool(ds) should raise a "the truth value of a Dataset is ambiguous" error 1090229430
1002534942 https://github.com/pydata/xarray/issues/6124#issuecomment-1002534942 https://api.github.com/repos/pydata/xarray/issues/6124 IC_kwDOAMm_X847wXge max-sixty 5635139 2021-12-29T10:53:53Z 2021-12-29T10:53:53Z MEMBER

I definitely empathize with the tradeoff here. That you found xarray's test's were making this error is fairly damning.

But the biggest impediment to changing this behavior is that Dataset follows the Mapping protocol, which has this behavior. One nice feature of xarray is that we follow python's protocols where possible, and that includes truthiness for dict-like / Mapping objects. Notably pd.DataFrame objects only partially implement the protocol, including .values.

If there's a synthesis of keeping the truthiness while reducing the chance of these mistakes, that would be very welcome.

I'm not sure this is an improvement, but in the example converting to a DataArray gets away from the truthiness issue: (result.min(...) >= 1.5).to_array().all()

(I wrote this before seeing @Illviljan 's response, which is very similar)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   bool(ds) should raise a "the truth value of a Dataset is ambiguous" error 1090229430

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