issue_comments

Table actions

10 rows where issue = 325609580 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 2

issue 1

  • Datasets more robust to non-string keys · 10 ✖

author_association 1

id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
392397697 https://github.com/pydata/xarray/pull/2174#issuecomment-392397697 https://api.github.com/repos/pydata/xarray/issues/2174 MDEyOklzc3VlQ29tbWVudDM5MjM5NzY5Nw== max-sixty 5635139 2018-05-28T01:43:57Z 2018-05-28T01:43:57Z MEMBER

thanks for working through the feedback, as ever, @shoyer ! It's like free coding classes

 
{
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 1,
    "rocket": 0,
    "eyes": 0
}
   Datasets more robust to non-string keys 325609580
392366040 https://github.com/pydata/xarray/pull/2174#issuecomment-392366040 https://api.github.com/repos/pydata/xarray/issues/2174 MDEyOklzc3VlQ29tbWVudDM5MjM2NjA0MA== shoyer 1217238 2018-05-27T20:48:40Z 2018-05-27T20:48:40Z MEMBER

thanks @maxim-lian !

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Datasets more robust to non-string keys 325609580
392111566 https://github.com/pydata/xarray/pull/2174#issuecomment-392111566 https://api.github.com/repos/pydata/xarray/issues/2174 MDEyOklzc3VlQ29tbWVudDM5MjExMTU2Ng== shoyer 1217238 2018-05-25T16:28:52Z 2018-05-25T16:48:15Z MEMBER

But I think just supporting this for isel/sel/reindex for now would be enough to be valuable. Note that currently it's on Dataset.reindex() but not DataArray.reindex().

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Datasets more robust to non-string keys 325609580
392103771 https://github.com/pydata/xarray/pull/2174#issuecomment-392103771 https://api.github.com/repos/pydata/xarray/issues/2174 MDEyOklzc3VlQ29tbWVudDM5MjEwMzc3MQ== shoyer 1217238 2018-05-25T15:59:50Z 2018-05-25T15:59:50Z MEMBER

Looking through Dataset/DataArray methods that support **kwargs, I think this may be the full list: - [ ] stack - [ ] shift - [ ] roll - [ ] set_index - [ ] reorder_levels - [ ] rolling - [ ] resample (not yet, we still support old behavior for the first positional arguments with a warning)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Datasets more robust to non-string keys 325609580
392083404 https://github.com/pydata/xarray/pull/2174#issuecomment-392083404 https://api.github.com/repos/pydata/xarray/issues/2174 MDEyOklzc3VlQ29tbWVudDM5MjA4MzQwNA== max-sixty 5635139 2018-05-25T14:53:22Z 2018-05-25T14:53:22Z MEMBER

AppVeyor failure unrelated

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Datasets more robust to non-string keys 325609580
392056416 https://github.com/pydata/xarray/pull/2174#issuecomment-392056416 https://api.github.com/repos/pydata/xarray/issues/2174 MDEyOklzc3VlQ29tbWVudDM5MjA1NjQxNg== max-sixty 5635139 2018-05-25T13:24:03Z 2018-05-25T13:24:03Z MEMBER

sel, isel & reindex are done

sel_points etc are deprecated so we can leave those

Any others on the 'first priority' list?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Datasets more robust to non-string keys 325609580
391957538 https://github.com/pydata/xarray/pull/2174#issuecomment-391957538 https://api.github.com/repos/pydata/xarray/issues/2174 MDEyOklzc3VlQ29tbWVudDM5MTk1NzUzOA== shoyer 1217238 2018-05-25T06:42:28Z 2018-05-25T06:42:28Z MEMBER

Arg name?

I'd like a convention of matching names to make it clear in docstrings that these are the same thing, e.g., - something_dict/**something, or - something/**something_kwargs

Probably the last is best since users will never have cause to type the longer argument name ending with _kwargs.

In practice, I suspect these first arguments are almost always going to be used positionally. We might even imagine making them positional only argument if PEP 570 is ever accepted. So I don't think it matters too much.

Combine or raise if supplied with kwargs?

Let's raise (like your current implementation) if both are provided. Combining could be error prone and seems unnecessarily complicated.

This is the same logic I implemented in the somewhat misleadingly named utils.combine_pos_and_kw_args() utility function (used by Dataset.reindex()).

Is there an encapsulation leak? Should we have a user API layer which is then translated to a more robust representation (i.e. kwargs to a dict), and those be separate methods? My initial inclination is that the current approach is good and managable

I agree -- I think the current version is manageable.

For internal use, we should always use the positional argument, but there's not much harm in leaving **kwargs around.

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Datasets more robust to non-string keys 325609580
391954557 https://github.com/pydata/xarray/pull/2174#issuecomment-391954557 https://api.github.com/repos/pydata/xarray/issues/2174 MDEyOklzc3VlQ29tbWVudDM5MTk1NDU1Nw== max-sixty 5635139 2018-05-25T06:25:18Z 2018-05-25T06:25:18Z MEMBER

@shoyer I see you're online - let me know if you have any thoughts re the changes. Am on a burst of open-source work atm

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Datasets more robust to non-string keys 325609580
391638302 https://github.com/pydata/xarray/pull/2174#issuecomment-391638302 https://api.github.com/repos/pydata/xarray/issues/2174 MDEyOklzc3VlQ29tbWVudDM5MTYzODMwMg== max-sixty 5635139 2018-05-24T08:47:46Z 2018-05-24T08:47:46Z MEMBER

DataArray.__getitem__ is only meant as a convenient short-cut for getting coordinates, which we might even want to deprecate entirely

👍 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Datasets more robust to non-string keys 325609580
391637781 https://github.com/pydata/xarray/pull/2174#issuecomment-391637781 https://api.github.com/repos/pydata/xarray/issues/2174 MDEyOklzc3VlQ29tbWVudDM5MTYzNzc4MQ== max-sixty 5635139 2018-05-24T08:45:59Z 2018-05-24T08:45:59Z MEMBER

I added an initial attempt at allowing for supplying a dict; on isel only at first

Let me know ppl's thoughts: - Arg name? - Combine or raise if supplied with kwargs? - Is there an encapsulation leak? Should we have a user API layer which is then translated to a more robust representation (i.e. kwargs to a dict), and those be separate methods? My initial inclination is that the current approach is good and managable

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Datasets more robust to non-string keys 325609580

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