home / github / issue_comments

Menu
  • Search all tables
  • GraphQL API

issue_comments: 391957538

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions performed_via_github_app issue
https://github.com/pydata/xarray/pull/2174#issuecomment-391957538 https://api.github.com/repos/pydata/xarray/issues/2174 391957538 MDEyOklzc3VlQ29tbWVudDM5MTk1NzUzOA== 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
}
  325609580
Powered by Datasette · Queries took 0.778ms · About: xarray-datasette