From 00821606b502c3b1d7401262b231b087d5393d64 Mon Sep 17 00:00:00 2001 From: Daniel Kirkham Date: Mon, 12 Mar 2018 17:53:23 +0000 Subject: [PATCH 1/5] Fix bug with aux coords with scalefactor/add_offset --- lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb | 4 ++-- lib/iris/fileformats/netcdf.py | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb b/lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb index e25a676122..3d62712eb9 100644 --- a/lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb +++ b/lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb @@ -1,4 +1,4 @@ -# (C) British Crown Copyright 2010 - 2017, Met Office +# (C) British Crown Copyright 2010 - 2018, Met Office # # This file is part of Iris. # @@ -1624,7 +1624,7 @@ fc_extras attr_units = get_attr_units(cf_coord_var, attributes) def cf_var_as_array(cf_var): - dtype = cf_var.dtype + dtype = iris.fileformats.netcdf.get_actual_dtype(cf_var) fill_value = getattr(cf_var.cf_data, '_FillValue', netCDF4.default_fillvals[dtype.str[1:]]) proxy = iris.fileformats.netcdf.NetCDFDataProxy( diff --git a/lib/iris/fileformats/netcdf.py b/lib/iris/fileformats/netcdf.py index 6ee830624b..ce1f5a9be1 100644 --- a/lib/iris/fileformats/netcdf.py +++ b/lib/iris/fileformats/netcdf.py @@ -490,8 +490,7 @@ def _set_attributes(attributes, key, value): attributes[str(key)] = value -def _load_cube(engine, cf, cf_var, filename): - """Create the cube associated with the CF-netCDF data variable.""" +def get_actual_dtype(cf_var): # Figure out what the eventual data type will be after any scale/offset # transforms. dummy_data = np.zeros(1, dtype=cf_var.dtype) @@ -499,12 +498,18 @@ def _load_cube(engine, cf, cf_var, filename): dummy_data = cf_var.scale_factor * dummy_data if hasattr(cf_var, 'add_offset'): dummy_data = cf_var.add_offset + dummy_data + return dummy_data.dtype + + +def _load_cube(engine, cf, cf_var, filename): + """Create the cube associated with the CF-netCDF data variable.""" + dtype = get_actual_dtype(cf_var) # Create cube with deferred data, but no metadata fill_value = getattr(cf_var.cf_data, '_FillValue', netCDF4.default_fillvals[cf_var.dtype.str[1:]]) - proxy = NetCDFDataProxy(cf_var.shape, dummy_data.dtype, - filename, cf_var.cf_name, fill_value) + proxy = NetCDFDataProxy(cf_var.shape, dtype, filename, cf_var.cf_name, + fill_value) data = as_lazy_data(proxy) cube = iris.cube.Cube(data) From c297aae075d78602d4ac2550d340d5ceb2f0370b Mon Sep 17 00:00:00 2001 From: Daniel Kirkham Date: Tue, 13 Mar 2018 11:49:50 +0000 Subject: [PATCH 2/5] Add scale_factor and add_offset to broken tests --- .../fc_rules_cf_fc/test_build_auxiliary_coordinate.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py b/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py index 69fa4ca643..9f04aad8ee 100644 --- a/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py +++ b/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py @@ -47,6 +47,8 @@ def setUp(self): units='m', shape=points.shape, dtype=points.dtype, + scale_factor=1, + add_offset=0, __getitem__=lambda self, key: points[key]) self.engine = mock.Mock( @@ -77,6 +79,8 @@ def test_slowest_varying_vertex_dim(self): cf_name='wibble_bnds', shape=bounds.shape, dtype=bounds.dtype, + scale_factor=1, + add_offset=0, __getitem__=lambda self, key: bounds[key]) # Expected bounds on the resulting coordinate should be rolled so that @@ -116,6 +120,8 @@ def test_fastest_varying_vertex_dim(self): cf_name='wibble_bnds', shape=bounds.shape, dtype=bounds.dtype, + scale_factor=1, + add_offset=0, __getitem__=lambda self, key: bounds[key]) expected_coord = AuxCoord( @@ -153,6 +159,8 @@ def test_fastest_with_different_dim_names(self): cf_name='wibble_bnds', shape=bounds.shape, dtype=bounds.dtype, + scale_factor=1, + add_offset=0, __getitem__=lambda self, key: bounds[key]) expected_coord = AuxCoord( From 7fb1344d9d82acc05fa08a4146c8a20d068c2854 Mon Sep 17 00:00:00 2001 From: Daniel Kirkham Date: Tue, 13 Mar 2018 12:13:19 +0000 Subject: [PATCH 3/5] License header --- .../fc_rules_cf_fc/test_build_auxiliary_coordinate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py b/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py index 9f04aad8ee..f7ec55321f 100644 --- a/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py +++ b/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py @@ -1,4 +1,4 @@ -# (C) British Crown Copyright 2014 - 2017, Met Office +# (C) British Crown Copyright 2014 - 2018, Met Office # # This file is part of Iris. # From 78ea0fbf3c5e3ccb40dad5d450d1fdb80c3d3392 Mon Sep 17 00:00:00 2001 From: Daniel Kirkham Date: Wed, 14 Mar 2018 13:35:47 +0000 Subject: [PATCH 4/5] Modify unit tests again and add specific dtype tests --- .../test_build_auxiliary_coordinate.py | 77 +++++++++++++++++-- 1 file changed, 69 insertions(+), 8 deletions(-) diff --git a/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py b/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py index f7ec55321f..a796621cc0 100644 --- a/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py +++ b/lib/iris/tests/unit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_auxiliary_coordinate.py @@ -30,6 +30,7 @@ import numpy as np from iris.coords import AuxCoord +from iris.fileformats.cf import CFVariable from iris.fileformats._pyke_rules.compiled_krb.fc_rules_cf_fc import \ build_auxiliary_coordinate from iris.tests import mock @@ -40,15 +41,15 @@ def setUp(self): # Create coordinate cf variables and pyke engine. points = np.arange(6).reshape(2, 3) self.cf_coord_var = mock.Mock( + spec=CFVariable, dimensions=('foo', 'bar'), cf_name='wibble', + cf_data=mock.Mock(), standard_name=None, long_name='wibble', units='m', shape=points.shape, dtype=points.dtype, - scale_factor=1, - add_offset=0, __getitem__=lambda self, key: points[key]) self.engine = mock.Mock( @@ -75,12 +76,12 @@ def test_slowest_varying_vertex_dim(self): # Create the bounds cf variable. bounds = np.arange(24).reshape(4, 2, 3) self.cf_bounds_var = mock.Mock( + spec=CFVariable, dimensions=('nv', 'foo', 'bar'), cf_name='wibble_bnds', + cf_data=mock.Mock(), shape=bounds.shape, dtype=bounds.dtype, - scale_factor=1, - add_offset=0, __getitem__=lambda self, key: bounds[key]) # Expected bounds on the resulting coordinate should be rolled so that @@ -116,12 +117,12 @@ def test_slowest_varying_vertex_dim(self): def test_fastest_varying_vertex_dim(self): bounds = np.arange(24).reshape(2, 3, 4) self.cf_bounds_var = mock.Mock( + spec=CFVariable, dimensions=('foo', 'bar', 'nv'), cf_name='wibble_bnds', + cf_data=mock.Mock(), shape=bounds.shape, dtype=bounds.dtype, - scale_factor=1, - add_offset=0, __getitem__=lambda self, key: bounds[key]) expected_coord = AuxCoord( @@ -155,12 +156,12 @@ def test_fastest_with_different_dim_names(self): # this should still work because the vertex dim is the fastest varying. bounds = np.arange(24).reshape(2, 3, 4) self.cf_bounds_var = mock.Mock( + spec=CFVariable, dimensions=('x', 'y', 'nv'), cf_name='wibble_bnds', + cf_data=mock.Mock(), shape=bounds.shape, dtype=bounds.dtype, - scale_factor=1, - add_offset=0, __getitem__=lambda self, key: bounds[key]) expected_coord = AuxCoord( @@ -189,5 +190,65 @@ def test_fastest_with_different_dim_names(self): expected_list) +class TestDtype(tests.IrisTest): + def setUp(self): + # Create coordinate cf variables and pyke engine. + points = np.arange(6).reshape(2, 3) + self.cf_coord_var = mock.Mock( + spec=CFVariable, + dimensions=('foo', 'bar'), + cf_name='wibble', + cf_data=mock.Mock(), + standard_name=None, + long_name='wibble', + units='m', + shape=points.shape, + dtype=points.dtype, + __getitem__=lambda self, key: points[key]) + + self.engine = mock.Mock( + cube=mock.Mock(), + cf_var=mock.Mock(dimensions=('foo', 'bar')), + filename='DUMMY', + provides=dict(coordinates=[])) + + def patched__getitem__(proxy_self, keys): + if proxy_self.variable_name == self.cf_coord_var.cf_name: + return self.cf_coord_var[keys] + raise RuntimeError() + + self.deferred_load_patch = mock.patch( + 'iris.fileformats.netcdf.NetCDFDataProxy.__getitem__', + new=patched__getitem__) + + def test_scale_factor_add_offset_int(self): + self.cf_coord_var.scale_factor = 3 + self.cf_coord_var.add_offset = 5 + + with self.deferred_load_patch: + build_auxiliary_coordinate(self.engine, self.cf_coord_var) + + coord, _ = self.engine.provides['coordinates'][0] + self.assertEqual(coord.dtype.kind, 'i') + + def test_scale_factor_float(self): + self.cf_coord_var.scale_factor = 3. + + with self.deferred_load_patch: + build_auxiliary_coordinate(self.engine, self.cf_coord_var) + + coord, _ = self.engine.provides['coordinates'][0] + self.assertEqual(coord.dtype.kind, 'f') + + def test_add_offset_float(self): + self.cf_coord_var.add_offset = 5. + + with self.deferred_load_patch: + build_auxiliary_coordinate(self.engine, self.cf_coord_var) + + coord, _ = self.engine.provides['coordinates'][0] + self.assertEqual(coord.dtype.kind, 'f') + + if __name__ == '__main__': tests.main() From a6efaea2d31d1d039d77fd2a0083266fb57a4f64 Mon Sep 17 00:00:00 2001 From: Daniel Kirkham Date: Thu, 15 Mar 2018 15:11:08 +0000 Subject: [PATCH 5/5] Make method private --- lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb | 2 +- lib/iris/fileformats/netcdf.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb b/lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb index 3d62712eb9..6274bef70b 100644 --- a/lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb +++ b/lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb @@ -1624,7 +1624,7 @@ fc_extras attr_units = get_attr_units(cf_coord_var, attributes) def cf_var_as_array(cf_var): - dtype = iris.fileformats.netcdf.get_actual_dtype(cf_var) + dtype = iris.fileformats.netcdf._get_actual_dtype(cf_var) fill_value = getattr(cf_var.cf_data, '_FillValue', netCDF4.default_fillvals[dtype.str[1:]]) proxy = iris.fileformats.netcdf.NetCDFDataProxy( diff --git a/lib/iris/fileformats/netcdf.py b/lib/iris/fileformats/netcdf.py index ce1f5a9be1..587b1e05f5 100644 --- a/lib/iris/fileformats/netcdf.py +++ b/lib/iris/fileformats/netcdf.py @@ -490,7 +490,7 @@ def _set_attributes(attributes, key, value): attributes[str(key)] = value -def get_actual_dtype(cf_var): +def _get_actual_dtype(cf_var): # Figure out what the eventual data type will be after any scale/offset # transforms. dummy_data = np.zeros(1, dtype=cf_var.dtype) @@ -503,7 +503,7 @@ def get_actual_dtype(cf_var): def _load_cube(engine, cf, cf_var, filename): """Create the cube associated with the CF-netCDF data variable.""" - dtype = get_actual_dtype(cf_var) + dtype = _get_actual_dtype(cf_var) # Create cube with deferred data, but no metadata fill_value = getattr(cf_var.cf_data, '_FillValue',