Skip to content

Reverse 2878 #2926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 19, 2017
Merged

Reverse 2878 #2926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ab05a7
Remove code introduced as workaround for a dask issue
djkirkham Dec 18, 2017
2ce0260
Fix issue where dask now returns MaskedConstant
djkirkham Dec 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 6 additions & 17 deletions lib/iris/_lazy_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,6 @@ def _limited_shape(shape):
return tuple(shape)


def _getall(a):
res = a[()]
if isinstance(res, ma.core.MaskedConstant):
res = ma.masked_array(res.data, mask=res.mask)
return res

_getall_delayed = dask.delayed(_getall)


def as_lazy_data(data, chunks=None, asarray=False):
"""
Convert the input array `data` to a dask array.
Expand Down Expand Up @@ -104,15 +95,10 @@ def as_lazy_data(data, chunks=None, asarray=False):
# but reduce it if larger than a default maximum size.
chunks = _limited_shape(data.shape)

if isinstance(data, ma.core.MaskedConstant):
data = ma.masked_array(data.data, mask=data.mask)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related, or fixing a different problem ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the way the code was before the original change. I had moved it into the _getall function, which I've now removed. I think it's necessary because dask doesn't like MaskedConstant.

if not is_lazy_data(data):
if data.shape == ():
# Workaround for https://github.com/dask/dask/issues/2823. Make
# sure scalar dask arrays return numpy objects.
dtype = data.dtype
data = _getall_delayed(data)
data = da.from_delayed(data, (), dtype)
else:
data = da.from_array(data, chunks=chunks, asarray=asarray)
data = da.from_array(data, chunks=chunks, asarray=asarray)
return data


Expand All @@ -138,7 +124,10 @@ def as_concrete_data(data):
# In some cases dask may return a scalar numpy.int/numpy.float object
# rather than a numpy.ndarray object.
# Recorded in https://github.com/dask/dask/issues/2111.
dtype = data.dtype
Copy link
Member

@pp-mo pp-mo Dec 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implication here is that data.dtype == data.compute().dtype is not guaranteed.
Is that really true + if so why ??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the result of the compute is a MaskedConstant, it will always have a float dtype

data = np.asanyarray(data.compute())
if isinstance(data, ma.core.MaskedConstant):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, how is this tied up with the main point ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I reverted the code to how it was before I found that some of our tests which check that the result of realising lazy data is not a MaskedConstant were failing. It seems that Dask is returning a MaskedConstant in situations where it wasn't before.

data = ma.masked_array(data.data, dtype=dtype, mask=data.mask)

return data

Expand Down
14 changes: 0 additions & 14 deletions lib/iris/tests/unit/lazy_data/test_as_concrete_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@
# importing anything else.
import iris.tests as tests

import unittest

import dask.array as da
import numpy as np
import numpy.ma as ma

Expand Down Expand Up @@ -93,17 +90,6 @@ def test_lazy_scalar_proxy_masked(self):
self.assertFalse(is_lazy_data(result))
self.assertMaskedArrayEqual(result, a)

def test_dask_scalar_proxy_pass_through(self):
# This test will fail when using a version of Dask with
# https://github.com/dask/dask/issues/2823 fixed. At that point the
# changes introduced in https://github.com/SciTools/iris/pull/2878 can
# be reversed.
a = np.array(5)
proxy = MyProxy(a)
d = da.from_array(proxy, 1, asarray=False)
result = d.compute()
self.assertEqual(proxy, result)


if __name__ == '__main__':
tests.main()