Fix decode cf with dask #2047
Fix decode cf with dask #2047
Conversation
| return Variable(dimensions, indexing.LazilyOuterIndexedArray(data), | ||
| attributes, encoding=encoding) | ||
| if not isinstance(data, dask_array_type): | ||
| data = indexing.LazilyOuterIndexedArray(data) |
There was a problem hiding this comment.
This is the offending line which caused the dask arrays to stop being dask arrays. I have simply bypassed it for dask arrays. This didn't break any tests. But can we think of any unforeseen negative consequences? What does LazilyOuterIndexedArray do here? I have to admit that I don't understand the details of our new indexing wrappers.
There was a problem hiding this comment.
LazilyOuterIndexedArray makes indexing on non-dask arrays lazy. It's not necessary or useful for dask arrays.
| 'y': ('t', [5, 10, -999], {'_FillValue': -999}) | ||
| }).chunk({'t': 1}) | ||
| decoded = conventions.decode_cf(original) | ||
| assert dask.is_dask_collection(decoded.y.data) |
There was a problem hiding this comment.
Is this an appropriate place to test for this? There is no other dask stuff in test_conventions.py, but it seemed to make sense there.
There was a problem hiding this comment.
This looks like a fine place to me.
|
Travis failures are for |
| 'y': ('t', [5, 10, -999], {'_FillValue': -999}) | ||
| }).chunk({'t': 1}) | ||
| decoded = conventions.decode_cf(original) | ||
| assert dask.is_dask_collection(decoded.y.data) |
There was a problem hiding this comment.
This looks like a fine place to me.
| return Variable(dimensions, indexing.LazilyOuterIndexedArray(data), | ||
| attributes, encoding=encoding) | ||
| if not isinstance(data, dask_array_type): | ||
| data = indexing.LazilyOuterIndexedArray(data) |
There was a problem hiding this comment.
LazilyOuterIndexedArray makes indexing on non-dask arrays lazy. It's not necessary or useful for dask arrays.
whats-new.rstfor all changes andapi.rstfor new APIThis was a very simple fix for an issue that has vexed me for quite a while. Am I missing something obvious here?