issue_comments

Table actions

4 rows where author_association = "MEMBER", issue = 1090229430 and user = 1217238 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

user 1

  • shoyer · 4 ✖

issue 1

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

author_association 1

  • MEMBER · 4 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1005918218 https://github.com/pydata/xarray/issues/6124#issuecomment-1005918218 https://api.github.com/repos/pydata/xarray/issues/6124 IC_kwDOAMm_X8479RgK shoyer 1217238 2022-01-05T17:16:56Z 2022-01-05T17:16:56Z MEMBER

I made a Twitter poll, let's see what that says 😄

https://twitter.com/xarray_dev/status/1478776987925684224?s=20

 
{
    "total_count": 1,
    "+1": 1,
    "-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
1005907803 https://github.com/pydata/xarray/issues/6124#issuecomment-1005907803 https://api.github.com/repos/pydata/xarray/issues/6124 IC_kwDOAMm_X8479O9b shoyer 1217238 2022-01-05T17:05:14Z 2022-01-05T17:05:14Z MEMBER

After discussing this a little more, I am on the fence about whether this change this is a good idea. If we can't come to concensus about expected behavior, then that is probably an indication that we should leave things the same to avoid churn.

 
{
    "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
1004474062 https://github.com/pydata/xarray/issues/6124#issuecomment-1004474062 https://api.github.com/repos/pydata/xarray/issues/6124 IC_kwDOAMm_X8473w7O shoyer 1217238 2022-01-04T01:59:46Z 2022-01-04T01:59:46Z MEMBER

@dcherian So I guess we decided to go for it? :)

@max-sixty We can still inherit from Mapping and just explicitly raise TypeError inside Dataset.__bool__ rather than using the default implementation. This is certainly a slight violation from the Mapping protocol, but xarray.Dataset is already a little inconsistent (e.g., we override __eq__ inconsistently with the default mapping behavior). Overall I think it's probably a worthwhile the more user-friendly choice?

 
{
    "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
1002746595 https://github.com/pydata/xarray/issues/6124#issuecomment-1002746595 https://api.github.com/repos/pydata/xarray/issues/6124 IC_kwDOAMm_X847xLLj shoyer 1217238 2021-12-29T19:30:29Z 2021-12-29T19:30:29Z MEMBER

The original intention here was definitely to honor the Mapping protocol exactly, but I agree that bool() is hard to use correctly on Dataset objects. I would support deprecating this.

 
{
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 1,
    "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]);