home / github / issue_comments

Menu
  • Search all tables
  • GraphQL API

issue_comments: 258623314

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/issues/1080#issuecomment-258623314 https://api.github.com/repos/pydata/xarray/issues/1080 258623314 MDEyOklzc3VlQ29tbWVudDI1ODYyMzMxNA== 941907 2016-11-05T16:41:07Z 2016-11-05T16:41:07Z NONE

Thank you for your response.

I still don't understand why you are pushing accessors in place of methods to such an extent. Is it because of namespace growth/conflicts? There are already many methods like diff, any which don't seem particularly more important than others. For instance, ndarray has no diff method yet you implement it.

While the solutions you presented are usable, they seem like workarounds and somewhat redundant or add extra like overhead (in terms of writing code). Registering extra dataset accessors where DataArray method application would do seems again redundant.

I would definitely discourage writing too many of such methods, though.

Could you please give some clear arguments why you discourage the use of normal methods? The two arguments listed in the docs don't really make a compelling case against method monkey-patching, because 1. name clashes can be easily checked for either approach (in either case you just check the existence of a class attribute) 2. caching on the dataset sometimes makes no sense and just adds redundancy and complicates the design and registering of extra functionality

I'm not trying to say that the accessor approach is wrong, I'm sure it makes sense for certain plugins. I'm just trying to share my experience with a very similar case where the simpler method approach turned out to be satisfactory and I think enabling it would increase the chances of more xarray plugins (which may not need accessor logic) coming to life.

Btw, perhaps it might be better to (perhaps optionally) issue a warning when overriding an existing class attribute during registering instead of completely refusing to do so.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  187373423
Powered by Datasette · Queries took 158.343ms · About: xarray-datasette