issue_comments: 490821558
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/2292#issuecomment-490821558 | https://api.github.com/repos/pydata/xarray/issues/2292 | 490821558 | MDEyOklzc3VlQ29tbWVudDQ5MDgyMTU1OA== | 47244312 | 2019-05-09T09:04:21Z | 2019-05-09T09:05:48Z | CONTRIBUTOR | There are problems with typing. I already mentioned them in #2929 but I'll summarize here. The vast majority of xarray functions/methods allow for "string or sequence of strings, optional". When you move to "hashable or sequence of hashables, optional", however, you want to specifically avoid tuples, which are both Sequence and Hashable instances. Most functions currently look like this:
One way to mitigate it would be to have an helper function, which would be invoked everywhere around the codebase, and then religiously make sure that the helper function is always used.
A completely separate problem with typing is that I expect a huge amount of xarray users to just assume variable names and dims are always strings. They'll have things like
The final problem is that integers are Hashables, and there's a wealth of cases in xarray where there is special logic that dynamically treats ints as positional indices. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
341643235 |