home / github

Menu
  • GraphQL API
  • Search all tables

issue_comments

Table actions
  • GraphQL API for issue_comments

3 rows where issue = 346294369 and user = 1217238 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

  • shoyer · 3 ✖

issue 1

  • Supplying a dataset to the dataset constructor · 3 ✖

author_association 1

  • MEMBER 3
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
422458598 https://github.com/pydata/xarray/pull/2330#issuecomment-422458598 https://api.github.com/repos/pydata/xarray/issues/2330 MDEyOklzc3VlQ29tbWVudDQyMjQ1ODU5OA== shoyer 1217238 2018-09-18T16:23:00Z 2018-09-18T16:23:00Z MEMBER

Sorry for taking so long here.

I'm still not entirely comfortable with this change -- it adds some additional complexity to the Dataset constructor that seems unnecessary. There's no inherit reason why calling a constructor on an object of the same type needs to work. If anything, I would like to deprecate/remove this behavior on DataArray (e.g., in favor of using a more explicit call to DataArray.copy(data=new_data)).

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

If I understand this correctly, you've written something like: python class MyDataset(xarray.Dataset): def __init__(self, dataset): super().__init__(dataset)

Is there any reason why you couldn't just write: python class MyDataset(xarray.Dataset): def __init__(self, dataset): super().__init__(dataset.data_vars, dataset.coords, dataset.attrs)

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

I don't think it's quite a coincidence here -- we are changing the behavior of Dataset.__iter__/Dataset.keys(), and this is a logical consequence of interpreting a Dataset as a mapping of its data variables :).

{
    "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
411258309 https://github.com/pydata/xarray/pull/2330#issuecomment-411258309 https://api.github.com/repos/pydata/xarray/issues/2330 MDEyOklzc3VlQ29tbWVudDQxMTI1ODMwOQ== shoyer 1217238 2018-08-08T01:56:04Z 2018-08-08T01:56:04Z MEMBER

Now I'm having second thoughts. It does make me a little nervous to add special cases that distinguish between xr.Dataset(ds, coords=some_coords) and xr.Dataset(ds.data_vars, coords=some_coords).

Maybe my proposed solution above is too complex, and it would be better simply to default the coords and attrs argument to those arguments from a passed Dataset object. This is basically what we do for DataArray.

That said, I do think we should try to resist the notion that every class constructor must also work for coercion to that class. This tends to result in a slightly confused interface. A separate as_dataset() function (even in user code) could give exactly the desired result in only be a few lines of code.

{
    "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
410046100 https://github.com/pydata/xarray/pull/2330#issuecomment-410046100 https://api.github.com/repos/pydata/xarray/issues/2330 MDEyOklzc3VlQ29tbWVudDQxMDA0NjEwMA== shoyer 1217238 2018-08-02T19:46:57Z 2018-08-02T19:46:57Z MEMBER

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

I think this would be slightly preferred, and also more consistent with the model of a Dataset as only iterating over data_vars. This suggests that at the very least, Dataset(ds) and Dataset(ds.data_vars) should be consistent after we finish the changes described in https://github.com/pydata/xarray/issues/884. Note that ds.data_vars will pull in coordinates associated with the DataArray objects.

To be more precise, I would probably make: ds2 = Dataset(ds1, coords, attrs) equivalent to ds2 = ds1.copy() ds2.coords.update(coords) ds2.attrs.update(attrs)

{
    "total_count": 2,
    "+1": 2,
    "-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]);
Powered by Datasette · Queries took 389.973ms · About: xarray-datasette