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 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.
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 |