issue_comments

Table actions

7 rows where issue = 346294369 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 · 7 ✖

issue 1

  • Supplying a dataset to the dataset constructor · 7 ✖

author_association 1

id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
424499776 https://github.com/pydata/xarray/pull/2330#issuecomment-424499776 https://api.github.com/repos/pydata/xarray/issues/2330 MDEyOklzc3VlQ29tbWVudDQyNDQ5OTc3Ng== max-sixty 5635139 2018-09-25T21:00:12Z 2018-09-25T21:00:12Z MEMBER

Yes, I agree - given the reluctance on the principle around "calling a constructor on an object of the same type needs to work" - it's right not to make this change. Thanks for holding that principle.

I'll close this; let me know if there's any change around this I make in its place.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Supplying a dataset to the dataset constructor 346294369
421494628 https://github.com/pydata/xarray/pull/2330#issuecomment-421494628 https://api.github.com/repos/pydata/xarray/issues/2330 MDEyOklzc3VlQ29tbWVudDQyMTQ5NDYyOA== max-sixty 5635139 2018-09-14T21:55:33Z 2018-09-14T21:55:33Z MEMBER

Any votes?

I think the current version dominates the current master (i.e. it doesn't disable any useful functionality, it adds some clarity). So I would vote to merge unless anyone has any alternative suggestions (which I will gladly implement)

Thanks team

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Supplying a dataset to the dataset constructor 346294369
414397606 https://github.com/pydata/xarray/pull/2330#issuecomment-414397606 https://api.github.com/repos/pydata/xarray/issues/2330 MDEyOklzc3VlQ29tbWVudDQxNDM5NzYwNg== max-sixty 5635139 2018-08-20T17:27:27Z 2018-08-20T17:27:27Z MEMBER

I think we should do something here, rather than change behavior by coincidence.

@shoyer do you want to make the decision?

I would vote +0.1 for the current version in the PR. V happy to be outweighed!

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Supplying a dataset to the dataset constructor 346294369
411461794 https://github.com/pydata/xarray/pull/2330#issuecomment-411461794 https://api.github.com/repos/pydata/xarray/issues/2330 MDEyOklzc3VlQ29tbWVudDQxMTQ2MTc5NA== max-sixty 5635139 2018-08-08T16:08:36Z 2018-08-08T16:08:36Z MEMBER

@fmaussion good question

The main reason I think this is worthwhile is the change in behavior between 0.10.x and 0.11.0, when iterating over a Dataset will iterate only over its data_vars, and so will change Dataset(ds) from ds to Dataset(ds.data_vars). I don't have a strong view on which option we choose, but I do think we should have a deliberate decision rather than making a breaking-change-by-coincidence.

To answer your question though, we hit this when inheriting from a Dataset: the object is initialized with a dataset, which is super-ed up to the Dataset constructor. (I realize this is narrow!). Alternatively, there is another repo that linked to the issue here

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Supplying a dataset to the dataset constructor 346294369
411114049 https://github.com/pydata/xarray/pull/2330#issuecomment-411114049 https://api.github.com/repos/pydata/xarray/issues/2330 MDEyOklzc3VlQ29tbWVudDQxMTExNDA0OQ== max-sixty 5635139 2018-08-07T16:14:57Z 2018-08-07T16:14:57Z MEMBER

And, to confirm, you think it's OK that :

Moving ahead with this, please let me know any objections from anyone

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Supplying a dataset to the dataset constructor 346294369
410122566 https://github.com/pydata/xarray/pull/2330#issuecomment-410122566 https://api.github.com/repos/pydata/xarray/issues/2330 MDEyOklzc3VlQ29tbWVudDQxMDEyMjU2Ng== max-sixty 5635139 2018-08-03T02:09:02Z 2018-08-03T02:09:02Z MEMBER

OK good!

And, to confirm, you think it's OK that :

xr.Dataset(ds, coords=some_coords).coords != some_coords

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Supplying a dataset to the dataset constructor 346294369
409618343 https://github.com/pydata/xarray/pull/2330#issuecomment-409618343 https://api.github.com/repos/pydata/xarray/issues/2330 MDEyOklzc3VlQ29tbWVudDQwOTYxODM0Mw== max-sixty 5635139 2018-08-01T15:35:53Z 2018-08-01T15:35:53Z MEMBER

Do we want to raise if coords / attrs are passed in addition to a Dataset? Do we want to warn for a couple versions first?

An alternative approach - more friendly but less explicit - is for any other argument (coords, attrs) supplied to overwrite the Dataset's

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Supplying a dataset to the dataset constructor 346294369

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