issue_comments

Table actions

9 rows where issue = 1503573351 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 3

author_association 2

issue 1

  • Follow keep_attrs in Dataset binary ops · 9 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1403869840 https://github.com/pydata/xarray/pull/7391#issuecomment-1403869840 https://api.github.com/repos/pydata/xarray/issues/7391 IC_kwDOAMm_X85TrVqQ dcherian 2448579 2023-01-25T16:17:30Z 2023-01-25T16:17:30Z MEMBER

Thanks @arongergely !

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Follow keep_attrs in Dataset binary ops 1503573351
1403706989 https://github.com/pydata/xarray/pull/7391#issuecomment-1403706989 https://api.github.com/repos/pydata/xarray/issues/7391 IC_kwDOAMm_X85Tqt5t arongergely 7316393 2023-01-25T14:27:15Z 2023-01-25T14:27:15Z CONTRIBUTOR

maybe not. I changed it back so you could squash to exclude the noise. The test is adjusted accordingly.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Follow keep_attrs in Dataset binary ops 1503573351
1384325872 https://github.com/pydata/xarray/pull/7391#issuecomment-1384325872 https://api.github.com/repos/pydata/xarray/issues/7391 IC_kwDOAMm_X85SgyLw arongergely 7316393 2023-01-16T16:58:15Z 2023-01-16T16:58:15Z CONTRIBUTOR

Will you cherry pick?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Follow keep_attrs in Dataset binary ops 1503573351
1384253560 https://github.com/pydata/xarray/pull/7391#issuecomment-1384253560 https://api.github.com/repos/pydata/xarray/issues/7391 IC_kwDOAMm_X85Sggh4 arongergely 7316393 2023-01-16T16:02:52Z 2023-01-16T16:03:02Z CONTRIBUTOR

Option 2 seems easier, so I'd suggest we go with that?

Agreed!

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Follow keep_attrs in Dataset binary ops 1503573351
1384168506 https://github.com/pydata/xarray/pull/7391#issuecomment-1384168506 https://api.github.com/repos/pydata/xarray/issues/7391 IC_kwDOAMm_X85SgLw6 keewis 14808389 2023-01-16T14:46:03Z 2023-01-16T14:46:03Z MEMBER

I guess this means that we have two options: just assume this does not cause anything else to fail to get this PR to work (but then this PR would be blocked until we fixed every use of the global keep_attrs in a separate PR), or we ignore str-valued / callable keep_attrs for now and merge your initial proposal.

Option 2 seems easier, so I'd suggest we go with that?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Follow keep_attrs in Dataset binary ops 1503573351
1384142858 https://github.com/pydata/xarray/pull/7391#issuecomment-1384142858 https://api.github.com/repos/pydata/xarray/issues/7391 IC_kwDOAMm_X85SgFgK arongergely 7316393 2023-01-16T14:24:23Z 2023-01-16T14:24:23Z CONTRIBUTOR

Changing the option setters to accept combine_attrs-style strings would open room for silent bugs: Other funcs/operators assume keep_attrs to be boolean so an if keep_attrs would cause trouble.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Follow keep_attrs in Dataset binary ops 1503573351
1384130795 https://github.com/pydata/xarray/pull/7391#issuecomment-1384130795 https://api.github.com/repos/pydata/xarray/issues/7391 IC_kwDOAMm_X85SgCjr arongergely 7316393 2023-01-16T14:16:27Z 2023-01-16T14:16:56Z CONTRIBUTOR

Hit a roadblock.

For binary ops the only way to set keep_attrs is through the options. However we can not set combine_attrs style strings to it due to strict validation. So this would lead to an error: python with xr.set_options(keep_attrs="drop_conflicts"): ds1 + ds2

To cricle around this we could let keep_attrs passed in the operator methods so user could do: python ds1.__add__(ds2, keep_attrs="drop_conflicts") But this isn't "nice".

Or perhaps I missed something? 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Follow keep_attrs in Dataset binary ops 1503573351
1378595020 https://github.com/pydata/xarray/pull/7391#issuecomment-1378595020 https://api.github.com/repos/pydata/xarray/issues/7391 IC_kwDOAMm_X85SK7DM keewis 14808389 2023-01-11T11:16:02Z 2023-01-11T11:16:02Z MEMBER

The missing documentation for this in apply_ufunc is definitely an oversight, so PRs very welcome!

consider this as new feature, not just a bugfix?

I always struggle with classifying changes, but I guess the more we add onto this PR, the more it becomes a new feature.

implement this for xarray.Variable too?

probably. The only reason this is not done yet is because nobody took the time to actually do it. So, once again: PRs very welcome!

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Follow keep_attrs in Dataset binary ops 1503573351
1378579381 https://github.com/pydata/xarray/pull/7391#issuecomment-1378579381 https://api.github.com/repos/pydata/xarray/issues/7391 IC_kwDOAMm_X85SK3O1 arongergely 7316393 2023-01-11T11:03:40Z 2023-01-11T11:03:40Z CONTRIBUTOR

Thanks for your suggestions @keewis!

If possible I'd like to change the computation of the attrs of the result a bit to allow more than just bool values (see below).

I was puzzled initially. We would introduce the combine_attrs behaviour of xarray.merge() but under keep_attrs. Then I found out this is already a thing in apply_ufunc().

I like the idea, shouldn't we - implement this for xarray.Variable too? - consider this as new feature, not just a bugfix? (for describing in whats-new.rst) - describe in the documentation? (also for apply_ufunc() - it's not yet described there).

could do all these, let me know

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Follow keep_attrs in Dataset binary ops 1503573351

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