issue_comments: 661226607

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/4221#issuecomment-661226607 https://api.github.com/repos/pydata/xarray/issues/4221 661226607 MDEyOklzc3VlQ29tbWVudDY2MTIyNjYwNw== 19578931 2020-07-20T17:35:31Z 2020-07-20T17:35:31Z CONTRIBUTOR

I have erred on the side of leaving the isinstance checks in place where @jthielen reviewed them. If it is decided that no further tests are needed, then this should be ready for review.

On another note, if it is appropriate to apply these changes:

Revisiting it I now realize that is_array_like is actually not a good name for that function: officially, "array_like" means that it can be casted using np.asarray. Maybe we should rename that to is_duck_array? But then we'd also need to check for __array_ufunc__ and __array_function__ because we also need those to properly wrap duck arrays.

As long as that doesn't break any of the current uses, I think that would be the best way forwards. This would require xarray to be on NumPy 1.16+ (in order to ensure hasattr(np.ndarray, "__array_function__"), but that's only 9 days away by NEP 29, so hopefully that isn't too much of an issue. Then, most of the checks for "can this be wrapped by xarray" should be able to become something like

python is_duck_array(x) and not isinstance(x, (DataArray, Variable))

with is_duck_dask_array only being needed for checking Dask Array-like support.

I can do that as well. If not, should a new issue be opened for this?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   656163384