Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def test_deferred_bytes(self):
field = mock.Mock(core_data=core_data)
data_shape = (100, 120)
proxy = mock.Mock(dtype=np.dtype('f4'), shape=data_shape,
spec=pp.PPDataProxy)
spec=pp.PPDataProxy, ndim=len(data_shape))
Copy link
Member

@pp-mo pp-mo Jul 11, 2019

Choose a reason for hiding this comment

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

OK, but I'm really surprised by this development.
Hitherto it was sufficient for a wrapped object to have just a .dtype and a .shape.
As ndim is a trivial consequence of shape, I wonder if this is a bit of a mistake in dask, and should be queried ??

Or am I missing something special about this particular item ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@pp-mo From what I understand there has been a bit of a refactor for dask version 2.0.0, so for whatever reason they have started to access the ndim attribute of dask.array like objects.

Perhaps raise your concerns with them in an issue, if you care, but I don't see this as a blocker to this PR. The horse has kinda bolted, as they have already cut dask version 2.0.0 and now 2.1.0.

Copy link
Member

Choose a reason for hiding this comment

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

FWIW, raised dask/dask#5106

Copy link
Member Author

Choose a reason for hiding this comment

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

@pp-mo I see that itch has been scratched 😜

Copy link
Member

@pp-mo pp-mo Jul 18, 2019

Choose a reason for hiding this comment

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

@bjlittle itch has been scratched
Close but no 🚬
⛈ in 🍵

# We can't directly inspect the concrete data source underlying
# the dask array, so instead we patch the proxy creation and check it's
# being created and invoked correctly.
Expand Down
3 changes: 2 additions & 1 deletion lib/iris/tests/unit/lazy_data/test_as_concrete_data.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# (C) British Crown Copyright 2017, Met Office
# (C) British Crown Copyright 2017 - 2019, Met Office
#
# This file is part of Iris.
#
Expand Down Expand Up @@ -34,6 +34,7 @@ class MyProxy(object):
def __init__(self, a):
self.shape = a.shape
self.dtype = a.dtype
self.ndim = a.ndim
self.a = a

def __getitem__(self, keys):
Expand Down
23 changes: 19 additions & 4 deletions lib/iris/tests/unit/lazy_data/test_co_realise_cubes.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# (C) British Crown Copyright 2018, Met Office
# (C) British Crown Copyright 2018 - 2019, Met Office
#
# This file is part of Iris.
#
Expand Down Expand Up @@ -36,6 +36,7 @@ class ArrayAccessCounter(object):
def __init__(self, array):
self.dtype = array.dtype
self.shape = array.shape
self.ndim = array.ndim
self._array = array
self.access_count = 0

Expand Down Expand Up @@ -71,15 +72,29 @@ def test_multi(self):
self.assertTrue(cube_inner.has_lazy_data())

def test_combined_access(self):
import dask
from distutils.version import StrictVersion as Version

wrapped_array = ArrayAccessCounter(np.arange(3.))
lazy_array = as_lazy_data(wrapped_array)
derived_a = lazy_array + 1
derived_b = lazy_array + 2
derived_c = lazy_array + 3
cube_a = Cube(derived_a)
cube_b = Cube(derived_b)
co_realise_cubes(cube_a, cube_b)
# Though used twice, the source data should only get fetched once.
self.assertEqual(wrapped_array.access_count, 1)
cube_c = Cube(derived_c)
co_realise_cubes(cube_a, cube_b, cube_c)
# Though used more than once, the source data should only get fetched
# once by dask.
if Version(dask.__version__) < Version('2.0.0'):
self.assertEqual(wrapped_array.access_count, 1)
else:
# dask 2+, now performs an initial data access with no data payload
# to ascertain the metadata associated with the dask.array, thus
# accounting for one additional access to the data from the
# perspective of this particular unit test.
# See dask.array.utils.meta_from_array
self.assertEqual(wrapped_array.access_count, 2)


if __name__ == '__main__':
Expand Down
2 changes: 1 addition & 1 deletion requirements/core.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ cartopy
#conda: proj4<5
cf-units>=2
cftime
dask[array]<2 #conda: dask<2
dask[array] #conda: dask
matplotlib>=2,<3
netcdf4
numpy>=1.14
Expand Down