issue_comments

Table actions

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

issue 1

  • Test suite: explicitly ignore irrelevant warnings · 9 ✖

author_association 1

id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
392650194 https://github.com/pydata/xarray/pull/2162#issuecomment-392650194 https://api.github.com/repos/pydata/xarray/issues/2162 MDEyOklzc3VlQ29tbWVudDM5MjY1MDE5NA== fujiisoup 6815844 2018-05-29T04:33:50Z 2018-05-29T04:33:50Z MEMBER

I just missed this line. Thanks. LGTM.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Test suite: explicitly ignore irrelevant warnings 324486173
392650084 https://github.com/pydata/xarray/pull/2162#issuecomment-392650084 https://api.github.com/repos/pydata/xarray/issues/2162 MDEyOklzc3VlQ29tbWVudDM5MjY1MDA4NA== shoyer 1217238 2018-05-29T04:32:44Z 2018-05-29T04:32:44Z MEMBER

This line only applies to non-Dataset objects (see the isintance check on the line above), so I think we are OK here.

On Mon, May 28, 2018 at 9:05 PM Keisuke Fujii notifications@github.com wrote:

@fujiisoup commented on this pull request.

In xarray/core/merge.py https://github.com/pydata/xarray/pull/2162#discussion_r191300053:

 """
 from .dataset import Dataset
 from .dataarray import DataArray
  • other = other.copy()
  • for k, obj in other.items():
  • if isinstance(obj, (Dataset, DataArray)):

  • drop duplicated coordinates

  • coord_names = [c for c in obj.coords
  • if c not in obj.dims and c in dataset.coords]
  • if coord_names:
  • other[k] = obj.drop(coord_names)
  • if not isinstance(other, Dataset):
  • other = OrderedDict(other)

I am slightly wondering this behavior may change after we finally support

884 https://github.com/pydata/xarray/issues/884, i.e. we will not

include coordinates in .key().

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/2162#pullrequestreview-123842932, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKS1gLkp5aD26C7wH18rk_zdfk9h5Kdks5t3Mj0gaJpZM4UFEGE . 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Test suite: explicitly ignore irrelevant warnings 324486173
392645688 https://github.com/pydata/xarray/pull/2162#issuecomment-392645688 https://api.github.com/repos/pydata/xarray/issues/2162 MDEyOklzc3VlQ29tbWVudDM5MjY0NTY4OA== shoyer 1217238 2018-05-29T03:54:35Z 2018-05-29T03:54:35Z MEMBER

I plan to merge this shortly (with a day or two) unless anyone else has comments

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Test suite: explicitly ignore irrelevant warnings 324486173
391937921 https://github.com/pydata/xarray/pull/2162#issuecomment-391937921 https://api.github.com/repos/pydata/xarray/issues/2162 MDEyOklzc3VlQ29tbWVudDM5MTkzNzkyMQ== shoyer 1217238 2018-05-25T04:30:21Z 2018-05-25T04:30:21Z MEMBER

Yes, if other is a Dataset coordinates get overwritten. We probably already had test coverage for this, but I also added some explicit tests. On Thu, May 24, 2018 at 9:21 PM Keisuke Fujii notifications@github.com wrote:

Do we update coordinates if other is Dataset?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/2162#issuecomment-391936961, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKS1v2WCnW59lZwnW_nc2oQftwaZTgVks5t14bNgaJpZM4UFEGE . 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Test suite: explicitly ignore irrelevant warnings 324486173
391936961 https://github.com/pydata/xarray/pull/2162#issuecomment-391936961 https://api.github.com/repos/pydata/xarray/issues/2162 MDEyOklzc3VlQ29tbWVudDM5MTkzNjk2MQ== fujiisoup 6815844 2018-05-25T04:21:33Z 2018-05-25T04:21:33Z MEMBER

Do we update coordinates if other is Dataset?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Test suite: explicitly ignore irrelevant warnings 324486173
391935599 https://github.com/pydata/xarray/pull/2162#issuecomment-391935599 https://api.github.com/repos/pydata/xarray/issues/2162 MDEyOklzc3VlQ29tbWVudDM5MTkzNTU5OQ== shoyer 1217238 2018-05-25T04:09:30Z 2018-05-25T04:09:30Z MEMBER

OK, updated to only silence the warning in Dataset.update(Dataset) (by refactoring the logic)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Test suite: explicitly ignore irrelevant warnings 324486173
391447131 https://github.com/pydata/xarray/pull/2162#issuecomment-391447131 https://api.github.com/repos/pydata/xarray/issues/2162 MDEyOklzc3VlQ29tbWVudDM5MTQ0NzEzMQ== shoyer 1217238 2018-05-23T18:16:03Z 2018-05-25T04:07:04Z MEMBER
  • [x] make a decision on coordinate conflicts with update()
 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Test suite: explicitly ignore irrelevant warnings 324486173
390630714 https://github.com/pydata/xarray/pull/2162#issuecomment-390630714 https://api.github.com/repos/pydata/xarray/issues/2162 MDEyOklzc3VlQ29tbWVudDM5MDYzMDcxNA== fujiisoup 6815844 2018-05-21T11:41:16Z 2018-05-23T00:57:05Z MEMBER

I didn't notice that iterating over dataset gives a FutureWarning.

Do you agree that we should have different behavior for overriding coords with setitem vs. update()?

I actually do not yet understand well the difference between ds.update(other) and ds.merge(other).

The docs says

If any dimensions would have inconsistent sizes in the updated dataset.update raises a ValueError,

but it does not. Maybe it should work identically with xr.merge(other)?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Test suite: explicitly ignore irrelevant warnings 324486173
390583609 https://github.com/pydata/xarray/pull/2162#issuecomment-390583609 https://api.github.com/repos/pydata/xarray/issues/2162 MDEyOklzc3VlQ29tbWVudDM5MDU4MzYwOQ== shoyer 1217238 2018-05-21T08:06:12Z 2018-05-21T08:06:12Z MEMBER

@fujiisoup any thoughts?

Do you agree that we should have different behavior for overriding coords with __setitem__ vs. update()?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Test suite: explicitly ignore irrelevant warnings 324486173

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