From 29752230a0b12288470e04fbf56f41d033ea7d64 Mon Sep 17 00:00:00 2001 From: Peter Killick Date: Wed, 25 Oct 2017 16:29:44 +0100 Subject: [PATCH 1/3] Importing Iris does not change dask state. --- lib/iris/_lazy_data.py | 22 +++++++--- .../unit/lazy_data/test_iris_dask_defaults.py | 41 ++++++++++++++++--- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/lib/iris/_lazy_data.py b/lib/iris/_lazy_data.py index b413d8752a..281e15ad49 100644 --- a/lib/iris/_lazy_data.py +++ b/lib/iris/_lazy_data.py @@ -31,6 +31,10 @@ import numpy.ma as ma +# A container for Iris defaults for dask lazy data processing. +DASK_OPTS = {} + + def _iris_dask_defaults(): """ Set dask defaults for Iris. The current default dask operation mode for @@ -42,17 +46,20 @@ def _iris_dask_defaults(): all available CPUs. .. note:: + We only want Iris to set dask options in the case where doing so will not change user-specified options that have already been set. """ + global DASK_OPTS if 'pool' not in dask.context._globals and \ 'get' not in dask.context._globals: - dask.set_options(get=dget_sync) - - -# Run this at import time to set dask options for Iris. -_iris_dask_defaults() + DASK_OPTS.update(get=dget_sync) + else: + # We may need to unset a previously-set default. + if DASK_OPTS.get('get') is not None: + DASK_OPTS = {key: value for key, value in DASK_OPTS.items() + if key != 'get'} def is_lazy_data(data): @@ -122,11 +129,14 @@ def as_concrete_data(data): """ if is_lazy_data(data): + # Check dask options at runtime to see if we need to set dask options + # for use in Iris. + _iris_dask_defaults() # Realise dask array, ensuring the data result is always a NumPy array. # 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. - data = np.asanyarray(data.compute()) + data = np.asanyarray(data.compute(**DASK_OPTS)) return data diff --git a/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py b/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py index cc9bbdfa3e..f6aaba1e5f 100644 --- a/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py +++ b/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py @@ -31,32 +31,61 @@ class Test__iris_dask_defaults(tests.IrisTest): def setUp(self): - set_options = 'dask.set_options' - self.patch_set_options = self.patch(set_options) + dask_opts = 'iris._lazy_data.DASK_OPTS' + self.mock_opts = self.patch(dask_opts, {}) self.mock_get_sync = tests.mock.sentinel.get_sync get_sync = 'iris._lazy_data.dget_sync' self.patch_get_sync = self.patch(get_sync, self.mock_get_sync) + self.iris_defaults = {'get': self.patch_get_sync} + + def test_startup(self): + # Dask options for Iris should not be modified on Iris import. + self.assertDictEqual(self.mock_opts, {}) def test_no_user_options(self): self.patch('dask.context._globals', {}) _iris_dask_defaults() - self.patch_set_options.assert_called_once_with(get=self.patch_get_sync) + self.assertDictEqual(self.mock_opts, self.iris_defaults) def test_user_options__pool(self): self.patch('dask.context._globals', {'pool': 5}) _iris_dask_defaults() - self.assertEqual(self.patch_set_options.call_count, 0) + self.assertDictEqual(self.mock_opts, {}) def test_user_options__get(self): self.patch('dask.context._globals', {'get': 'threaded'}) _iris_dask_defaults() - self.assertEqual(self.patch_set_options.call_count, 0) + self.assertDictEqual(self.mock_opts, {}) def test_user_options__wibble(self): # Test a user-specified dask option that does not affect Iris. self.patch('dask.context._globals', {'wibble': 'foo'}) _iris_dask_defaults() - self.patch_set_options.assert_called_once_with(get=self.patch_get_sync) + self.assertDictEqual(self.mock_opts, self.iris_defaults) + + def test_changed_options__add(self): + # Check that adding dask options during a session alters Iris dask + # processing options. + # Starting condition: no dask options set. + self.patch('dask.context._globals', {}) + _iris_dask_defaults() + self.assertDictEqual(self.mock_opts, self.iris_defaults) + # Updated condition: dask option is set. + self.patch('dask.context._globals', {'get': 'threaded'}) + _iris_dask_defaults() + self.assertDictEqual(self.mock_opts, {}) + + def test_changed_options__remove(self): + # Check that removing dask options during a session alters Iris dask + # processing options. + # Starting condition: dask option is set. + self.patch('dask.context._globals', {'get': 'threaded'}) + _iris_dask_defaults() + self.assertDictEqual(self.mock_opts, {}) + # Updated condition: no dask options set. + self.patch('dask.context._globals', {}) + _iris_dask_defaults() + self.assertDictEqual(self.mock_opts, self.iris_defaults) if __name__ == '__main__': From 2d58953dabae0d3813cfc2a99cec41a6225d89b0 Mon Sep 17 00:00:00 2001 From: Peter Killick Date: Thu, 26 Oct 2017 16:51:15 +0100 Subject: [PATCH 2/3] Review actions: no globals and passing tests \\o/ --- lib/iris/_lazy_data.py | 17 +++--- .../unit/lazy_data/test_iris_dask_defaults.py | 58 +++++++++---------- 2 files changed, 33 insertions(+), 42 deletions(-) diff --git a/lib/iris/_lazy_data.py b/lib/iris/_lazy_data.py index 281e15ad49..a0315caeb4 100644 --- a/lib/iris/_lazy_data.py +++ b/lib/iris/_lazy_data.py @@ -31,10 +31,6 @@ import numpy.ma as ma -# A container for Iris defaults for dask lazy data processing. -DASK_OPTS = {} - - def _iris_dask_defaults(): """ Set dask defaults for Iris. The current default dask operation mode for @@ -51,15 +47,16 @@ def _iris_dask_defaults(): not change user-specified options that have already been set. """ - global DASK_OPTS + dask_opts = {} if 'pool' not in dask.context._globals and \ 'get' not in dask.context._globals: - DASK_OPTS.update(get=dget_sync) + dask_opts.update(get=dget_sync) else: # We may need to unset a previously-set default. - if DASK_OPTS.get('get') is not None: - DASK_OPTS = {key: value for key, value in DASK_OPTS.items() + if dask_opts.get('get') is not None: + dask_opts = {key: value for key, value in dask_opts.items() if key != 'get'} + return dask_opts def is_lazy_data(data): @@ -131,12 +128,12 @@ def as_concrete_data(data): if is_lazy_data(data): # Check dask options at runtime to see if we need to set dask options # for use in Iris. - _iris_dask_defaults() + dask_opts = _iris_dask_defaults() # Realise dask array, ensuring the data result is always a NumPy array. # 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. - data = np.asanyarray(data.compute(**DASK_OPTS)) + data = np.asanyarray(data.compute(**dask_opts)) return data diff --git a/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py b/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py index f6aaba1e5f..cce07ca557 100644 --- a/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py +++ b/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py @@ -31,61 +31,55 @@ class Test__iris_dask_defaults(tests.IrisTest): def setUp(self): - dask_opts = 'iris._lazy_data.DASK_OPTS' - self.mock_opts = self.patch(dask_opts, {}) self.mock_get_sync = tests.mock.sentinel.get_sync get_sync = 'iris._lazy_data.dget_sync' self.patch_get_sync = self.patch(get_sync, self.mock_get_sync) self.iris_defaults = {'get': self.patch_get_sync} - def test_startup(self): - # Dask options for Iris should not be modified on Iris import. - self.assertDictEqual(self.mock_opts, {}) - def test_no_user_options(self): - self.patch('dask.context._globals', {}) - _iris_dask_defaults() - self.assertDictEqual(self.mock_opts, self.iris_defaults) + with tests.mock.patch('dask.context._globals', {}): + opts = _iris_dask_defaults() + self.assertDictEqual(opts, self.iris_defaults) def test_user_options__pool(self): - self.patch('dask.context._globals', {'pool': 5}) - _iris_dask_defaults() - self.assertDictEqual(self.mock_opts, {}) + with tests.mock.patch('dask.context._globals', {'pool': 5}): + opts = _iris_dask_defaults() + self.assertDictEqual(opts, {}) def test_user_options__get(self): - self.patch('dask.context._globals', {'get': 'threaded'}) - _iris_dask_defaults() - self.assertDictEqual(self.mock_opts, {}) + with tests.mock.patch('dask.context._globals', {'get': 'threaded'}): + opts = _iris_dask_defaults() + self.assertDictEqual(opts, {}) def test_user_options__wibble(self): # Test a user-specified dask option that does not affect Iris. - self.patch('dask.context._globals', {'wibble': 'foo'}) - _iris_dask_defaults() - self.assertDictEqual(self.mock_opts, self.iris_defaults) + with tests.mock.patch('dask.context._globals', {'wibble': 'foo'}): + opts = _iris_dask_defaults() + self.assertDictEqual(opts, self.iris_defaults) def test_changed_options__add(self): # Check that adding dask options during a session alters Iris dask # processing options. # Starting condition: no dask options set. - self.patch('dask.context._globals', {}) - _iris_dask_defaults() - self.assertDictEqual(self.mock_opts, self.iris_defaults) - # Updated condition: dask option is set. - self.patch('dask.context._globals', {'get': 'threaded'}) - _iris_dask_defaults() - self.assertDictEqual(self.mock_opts, {}) + with tests.mock.patch('dask.context._globals', {}): + opts = _iris_dask_defaults() + self.assertDictEqual(opts, self.iris_defaults) + # Updated condition: dask option is set. + with tests.mock.patch('dask.context._globals', {'pool': 5}): + opts = _iris_dask_defaults() + self.assertDictEqual(opts, {}) def test_changed_options__remove(self): # Check that removing dask options during a session alters Iris dask # processing options. # Starting condition: dask option is set. - self.patch('dask.context._globals', {'get': 'threaded'}) - _iris_dask_defaults() - self.assertDictEqual(self.mock_opts, {}) - # Updated condition: no dask options set. - self.patch('dask.context._globals', {}) - _iris_dask_defaults() - self.assertDictEqual(self.mock_opts, self.iris_defaults) + with tests.mock.patch('dask.context._globals', {'get': 'threaded'}): + opts = _iris_dask_defaults() + self.assertDictEqual(opts, {}) + # Updated condition: no dask options set. + with tests.mock.patch('dask.context._globals', {}): + opts = _iris_dask_defaults() + self.assertDictEqual(opts, self.iris_defaults) if __name__ == '__main__': From ae88bc7635da156d2415f22553ad33a772cc122d Mon Sep 17 00:00:00 2001 From: Peter Killick Date: Thu, 26 Oct 2017 17:23:13 +0100 Subject: [PATCH 3/3] Review actions II: make external private API access more robust --- lib/iris/_lazy_data.py | 18 ++++++++++-------- .../unit/lazy_data/test_iris_dask_defaults.py | 6 ++++++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/iris/_lazy_data.py b/lib/iris/_lazy_data.py index a0315caeb4..f940216051 100644 --- a/lib/iris/_lazy_data.py +++ b/lib/iris/_lazy_data.py @@ -48,14 +48,16 @@ def _iris_dask_defaults(): """ dask_opts = {} - if 'pool' not in dask.context._globals and \ - 'get' not in dask.context._globals: - dask_opts.update(get=dget_sync) - else: - # We may need to unset a previously-set default. - if dask_opts.get('get') is not None: - dask_opts = {key: value for key, value in dask_opts.items() - if key != 'get'} + dask_globals = getattr(dask.context, '_globals') + if dask_globals is not None: + if 'pool' not in dask_globals and \ + 'get' not in dask_globals: + dask_opts.update(get=dget_sync) + else: + # We may need to unset a previously-set default. + if dask_opts.get('get') is not None: + dask_opts = {key: value for key, value in dask_opts.items() + if key != 'get'} return dask_opts diff --git a/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py b/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py index cce07ca557..5b9a515a63 100644 --- a/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py +++ b/lib/iris/tests/unit/lazy_data/test_iris_dask_defaults.py @@ -25,6 +25,7 @@ # Import iris.tests first so that some things can be initialised before # importing anything else. import iris.tests as tests +import dask.context from iris._lazy_data import _iris_dask_defaults @@ -36,6 +37,11 @@ def setUp(self): self.patch_get_sync = self.patch(get_sync, self.mock_get_sync) self.iris_defaults = {'get': self.patch_get_sync} + def test_dask_context_api(self): + # A first line of defence to check `dask.context._globals` + # still exists. + self.assertTrue(hasattr(dask.context, '_globals')) + def test_no_user_options(self): with tests.mock.patch('dask.context._globals', {}): opts = _iris_dask_defaults()