issue_comments

Table actions

10 rows where author_association = "MEMBER", issue = 571743567 and user = 5635139 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 1

  • max-sixty · 10 ✖

issue 1

  • Coarsen keep attrs 3376 · 10 ✖

author_association 1

  • MEMBER · 10 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
593668722 https://github.com/pydata/xarray/pull/3801#issuecomment-593668722 https://api.github.com/repos/pydata/xarray/issues/3801 MDEyOklzc3VlQ29tbWVudDU5MzY2ODcyMg== max-sixty 5635139 2020-03-02T23:03:19Z 2020-03-02T23:03:19Z MEMBER

Thanks a lot @amcnicho ! Glad to have you as a contributor!

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Coarsen keep attrs 3376 571743567
593486987 https://github.com/pydata/xarray/pull/3801#issuecomment-593486987 https://api.github.com/repos/pydata/xarray/issues/3801 MDEyOklzc3VlQ29tbWVudDU5MzQ4Njk4Nw== max-sixty 5635139 2020-03-02T16:23:01Z 2020-03-02T16:23:01Z MEMBER

Looks great! Any feedback from others? Otherwise will merge later today

Thanks @amcnicho !

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Coarsen keep attrs 3376 571743567
592986729 https://github.com/pydata/xarray/pull/3801#issuecomment-592986729 https://api.github.com/repos/pydata/xarray/issues/3801 MDEyOklzc3VlQ29tbWVudDU5Mjk4NjcyOQ== max-sixty 5635139 2020-02-29T19:40:41Z 2020-02-29T19:40:41Z MEMBER

This looks excellent!

Could we add the change to whatsnew.rst?

Then I think we're ready to merge!

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Coarsen keep attrs 3376 571743567
592288706 https://github.com/pydata/xarray/pull/3801#issuecomment-592288706 https://api.github.com/repos/pydata/xarray/issues/3801 MDEyOklzc3VlQ29tbWVudDU5MjI4ODcwNg== max-sixty 5635139 2020-02-28T03:06:42Z 2020-02-28T03:06:42Z MEMBER

That's looking good re coarsen! Do tests pass?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Coarsen keep attrs 3376 571743567
592219280 https://github.com/pydata/xarray/pull/3801#issuecomment-592219280 https://api.github.com/repos/pydata/xarray/issues/3801 MDEyOklzc3VlQ29tbWVudDU5MjIxOTI4MA== max-sixty 5635139 2020-02-27T22:54:06Z 2020-02-27T22:54:06Z MEMBER

@amcnicho

  • Your current Rolling implementation looks good. Does that work?
  • For Coarsen, adding keep_attrs to the constructor is right, and then it needs to be assigned it to the object
  • Unlike the Rolling implementation, you can add the logic for keeping attrs within the wrapped_func (i.e. here ), by branching on the value of self.keep_attrs; self there is the Coarsen object.

Does that make sense?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Coarsen keep attrs 3376 571743567
592171368 https://github.com/pydata/xarray/pull/3801#issuecomment-592171368 https://api.github.com/repos/pydata/xarray/issues/3801 MDEyOklzc3VlQ29tbWVudDU5MjE3MTM2OA== max-sixty 5635139 2020-02-27T20:53:06Z 2020-02-27T22:51:49Z MEMBER

Right, yes, this does get a bit nested... Appreciate you working through it.

At first glance, _reduce_method works differently in Rolling & Coarsen. You can consider leaving Rolling and just focusing on Coarsen.

~I think you want to catch the kwarg here~ and use it to decide whether to include the attrs here. It should not be a kwarg into wrapped_func.

Does that make sense?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Coarsen keep attrs 3376 571743567
592172830 https://github.com/pydata/xarray/pull/3801#issuecomment-592172830 https://api.github.com/repos/pydata/xarray/issues/3801 MDEyOklzc3VlQ29tbWVudDU5MjE3MjgzMA== max-sixty 5635139 2020-02-27T20:56:49Z 2020-02-27T20:56:49Z MEMBER

...but then potentially it does conflict with the existing use of _reduce_method. I can have more of a look tonight.

CC @fujiisoup if you're more familiar with this code. And @dcherian has done some work on it.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Coarsen keep attrs 3376 571743567
592145566 https://github.com/pydata/xarray/pull/3801#issuecomment-592145566 https://api.github.com/repos/pydata/xarray/issues/3801 MDEyOklzc3VlQ29tbWVudDU5MjE0NTU2Ng== max-sixty 5635139 2020-02-27T19:49:37Z 2020-02-27T19:49:37Z MEMBER

(sorry, I didn't see your message before my message prior)

Do you know what's calling nanmean with keep_attrs? I think should be handling keep_attrs before we get to those numerical functions on variables.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Coarsen keep attrs 3376 571743567
592136711 https://github.com/pydata/xarray/pull/3801#issuecomment-592136711 https://api.github.com/repos/pydata/xarray/issues/3801 MDEyOklzc3VlQ29tbWVudDU5MjEzNjcxMQ== max-sixty 5635139 2020-02-27T19:30:18Z 2020-02-27T19:30:18Z MEMBER

Recent commits look good @amcnicho !

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Coarsen keep attrs 3376 571743567
591740060 https://github.com/pydata/xarray/pull/3801#issuecomment-591740060 https://api.github.com/repos/pydata/xarray/issues/3801 MDEyOklzc3VlQ29tbWVudDU5MTc0MDA2MA== max-sixty 5635139 2020-02-27T02:05:37Z 2020-02-27T02:05:37Z MEMBER

Thanks for the PR and tests @amcnicho . Are you up for adding the code to pass through keep_attrs?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Coarsen keep attrs 3376 571743567

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