From bedb1f5e3c81a4fd92e4c793588380b31bf6e048 Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Wed, 25 Sep 2024 18:05:28 +1000 Subject: [PATCH 01/15] Refactor: replace cube fakes with DummyCube. --- test/test_um2netcdf.py | 115 ++++++++++++++--------------------------- 1 file changed, 40 insertions(+), 75 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index f258a28..9fa1e2d 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -70,46 +70,28 @@ def mule_vars(z_sea_rho_data, z_sea_theta_data): return um2nc.MuleVars(um2nc.GRID_NEW_DYNAMICS, d_lat, d_lon, z_sea_rho_data, z_sea_theta_data) -def set_default_attrs(cube, item_code: int, var_name: str): - """Add subset of default attributes to flesh out cube like objects.""" - cube.__dict__.update({"item_code": item_code, - "var_name": var_name, - "long_name": "", - "coord": {"latitude": 0.0, # TODO: real val = ? - "longitude": 0.0}, # TODO: real val - "cell_methods": [], - "data": None, - }) - - section, item = um2nc.to_stash_code(item_code) - cube.attributes = {um2nc.STASH: DummyStash(section, item)} - - @pytest.fixture -def air_temp_cube(): - # data copied from aiihca.paa1jan.subset file - name = "air_temperature" - m_air_temp = mock.NonCallableMagicMock(spec=iris.cube.Cube, name=name) - set_default_attrs(m_air_temp, 30204, name) - return m_air_temp +def air_temp_cube(lat_river_coord, lon_river_coord): + # details copied from aiihca.paa1jan.subset file + air_temp = DummyCube(30204, "air_temperature", + coords=[lat_river_coord, lon_river_coord]) + return air_temp @pytest.fixture -def precipitation_flux_cube(): +def precipitation_flux_cube(lat_river_coord, lon_river_coord): # copied from aiihca.paa1jan.subset file - name = "precipitation_flux" - m_flux = mock.NonCallableMagicMock(spec=iris.cube.Cube, name=name) - set_default_attrs(m_flux, 5216, name) - return m_flux + precipitation_flux = DummyCube(5216, "precipitation_flux", + coords=[lat_river_coord, lon_river_coord]) + return precipitation_flux # create cube requiring heaviside_t masking @pytest.fixture -def geo_potential_cube(): - name = "geopotential_height" - m_geo_potential = mock.NonCallableMagicMock(spec=iris.cube.Cube, name=name) - set_default_attrs(m_geo_potential, 30297, name) - return m_geo_potential +def geo_potential_cube(lat_river_coord, lon_river_coord): + geo_potential = DummyCube(30297, "geopotential_height", + coords=[lat_river_coord, lon_river_coord]) + return geo_potential @pytest.fixture @@ -151,14 +133,8 @@ def test_process_no_heaviside_drop_cubes(air_temp_cube, precipitation_flux_cube, # use mocks to prevent mule data extraction file I/O mock.patch("mule.load_umfile"), mock.patch("umpost.um2netcdf.process_mule_vars") as m_mule_vars, - mock.patch("iris.load") as m_iris_load, mock.patch("iris.fileformats.netcdf.Saver") as m_saver, # prevent I/O - - # TODO: lat/long & level coord fixes require more internal data attrs - # skip temporarily to manage test complexity - mock.patch("umpost.um2netcdf.fix_latlon_coords"), - mock.patch("umpost.um2netcdf.fix_level_coord"), mock.patch("umpost.um2netcdf.cubewrite"), ): m_mule_vars.return_value = mule_vars @@ -193,7 +169,6 @@ def test_process_all_cubes_filtered(air_temp_cube, geo_potential_cube, with ( mock.patch("mule.load_umfile"), mock.patch("umpost.um2netcdf.process_mule_vars") as m_mule_vars, - mock.patch("iris.load") as m_iris_load, mock.patch("iris.fileformats.netcdf.Saver") as m_saver, # prevent I/O ): @@ -213,11 +188,8 @@ def test_process_mask_with_heaviside(air_temp_cube, precipitation_flux_cube, with ( mock.patch("mule.load_umfile"), mock.patch("umpost.um2netcdf.process_mule_vars") as m_mule_vars, - mock.patch("iris.load") as m_iris_load, mock.patch("iris.fileformats.netcdf.Saver") as m_saver, # prevent I/O - mock.patch("umpost.um2netcdf.fix_latlon_coords"), - mock.patch("umpost.um2netcdf.fix_level_coord"), mock.patch("umpost.um2netcdf.apply_mask"), # TODO: eventually call real version mock.patch("umpost.um2netcdf.cubewrite"), ): @@ -253,12 +225,8 @@ def test_process_no_masking_keep_all_cubes(air_temp_cube, precipitation_flux_cub with ( mock.patch("mule.load_umfile"), mock.patch("umpost.um2netcdf.process_mule_vars") as m_mule_vars, - mock.patch("iris.load") as m_iris_load, mock.patch("iris.fileformats.netcdf.Saver") as m_saver, # prevent I/O - - mock.patch("umpost.um2netcdf.fix_latlon_coords"), - mock.patch("umpost.um2netcdf.fix_level_coord"), mock.patch("umpost.um2netcdf.cubewrite"), ): m_mule_vars.return_value = mule_vars @@ -368,22 +336,13 @@ def add_stash(cube, stash): setattr(cube, "attributes", d) -@dataclass() -class PartialCube: - # work around mocks & DummyCube having item_code attr - var_name: str - attributes: dict - standard_name: str = None - long_name: str = None - - def test_set_item_codes(): - cube0 = PartialCube("d0", {um2nc.STASH: DummyStash(1, 2)}) - cube1 = PartialCube("d1", {um2nc.STASH: DummyStash(3, 4)}) + cube0 = DummyCube(1002, "d0", {um2nc.STASH: DummyStash(1, 2)}) + cube1 = DummyCube(3004, "d1", {um2nc.STASH: DummyStash(3, 4)}) cubes = [cube0, cube1] for cube in cubes: - assert not hasattr(cube, um2nc.ITEM_CODE) + assert hasattr(cube, um2nc.ITEM_CODE) um2nc.set_item_codes(cubes) c0, c1 = cubes @@ -404,21 +363,25 @@ def __init__(self, item_code, var_name=None, attributes=None, self.attributes = attributes self.units = units self.standard_name = None - self.long_name = None + self.long_name = "" + self.cell_methods = [] self.data = None # Mimic a coordinate dictionary keys for iris coordinate names. This # ensures the access key for coord() matches the coordinate's name self._coordinates = {c.name(): c for c in coords} if coords else {} + section, item = um2nc.to_stash_code(item_code) + self.attributes = {um2nc.STASH: DummyStash(section, item)} + def name(self): return self.var_name - def coord(self, name): + def coord(self, _name): try: - return self._coordinates[name] + return self._coordinates[_name] except KeyError: - msg = f"{self.__class__}: lacks coord for '{name}'" + msg = f"{self.__class__}[{self.var_name}]: lacks coord for '{_name}'" raise CoordinateNotFoundError(msg) @@ -438,18 +401,21 @@ def ua_plev_cube(): @pytest.fixture -def heaviside_uv_cube(): - return DummyCube(30301, "heaviside_uv") +def heaviside_uv_cube(lat_river_coord, lon_river_coord): + return DummyCube(30301, "heaviside_uv", + coords=[lat_river_coord, lon_river_coord]) @pytest.fixture -def ta_plev_cube(): - return DummyCube(30294, "ta_plev") +def ta_plev_cube(lat_river_coord, lon_river_coord): + return DummyCube(30294, "ta_plev", + coords=[lat_river_coord, lon_river_coord]) @pytest.fixture -def heaviside_t_cube(): - return DummyCube(30304, "heaviside_t") +def heaviside_t_cube(lat_river_coord, lon_river_coord): + return DummyCube(30304, "heaviside_t", + coords=[lat_river_coord, lon_river_coord]) # cube filtering tests @@ -486,13 +452,11 @@ def test_cube_filtering_no_include_exclude(ua_plev_cube, heaviside_uv_cube): # cube variable renaming tests @pytest.fixture def x_wind_cube(): - fake_cube = PartialCube("var_name", {'STASH': DummyStash(0, 2)}, "x_wind") - fake_cube.cell_methods = [] - return fake_cube - - -# UMStash = namedtuple("UMStash", -# "long_name, name, units, standard_name, uniquename") + x_wind_cube = DummyCube(2, var_name="var_name", + attributes={'STASH': DummyStash(0, 2)}) + x_wind_cube.standard_name = "x_wind" + x_wind_cube.cell_methods = [] + return x_wind_cube CellMethod = namedtuple("CellMethod", "method") @@ -553,7 +517,8 @@ def test_fix_standard_name_update_x_wind(x_wind_cube): def test_fix_standard_name_update_y_wind(): # test cube wind renaming block only # use empty std name to bypass renaming logic - m_cube = PartialCube("var_name", {'STASH': DummyStash(0, 3)}, "y_wind") + m_cube = DummyCube(3, attributes={'STASH': DummyStash(0, 3)}) + m_cube.standard_name = "y_wind" m_cube.cell_methods = [] um2nc.fix_standard_name(m_cube, "", verbose=False) From 80646c8188444814f3c05db25b2704cf1939daa7 Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Thu, 26 Sep 2024 14:52:06 +1000 Subject: [PATCH 02/15] Minor fixes of comments & docstrings. --- test/test_um2netcdf.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index 9fa1e2d..f51589d 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -86,9 +86,9 @@ def precipitation_flux_cube(lat_river_coord, lon_river_coord): return precipitation_flux -# create cube requiring heaviside_t masking @pytest.fixture def geo_potential_cube(lat_river_coord, lon_river_coord): + """Return new cube requiring heaviside_t masking""" geo_potential = DummyCube(30297, "geopotential_height", coords=[lat_river_coord, lon_river_coord]) return geo_potential @@ -120,15 +120,15 @@ def fake_out_path(): return "/tmp-does-not-exist/fake_input_fields_file.nc" +# FIXME: the convoluted setup in test_process_...() is a code smell +# use the following tests to gradually refactor process() +# TODO: evolve towards design where input & output file I/O is extracted from +# process() & the function only takes *raw data only* (is highly testable) + def test_process_no_heaviside_drop_cubes(air_temp_cube, precipitation_flux_cube, geo_potential_cube, mule_vars, std_args, fake_in_path, fake_out_path): """Attempt end-to-end process() test, dropping cubes requiring masking.""" - - # FIXME: this convoluted setup is a code smell - # use these tests to gradually refactor process() - # TODO: move towards a design where input & output I/O is extracted from process() - # process() should eventually operate on *data only* args with ( # use mocks to prevent mule data extraction file I/O mock.patch("mule.load_umfile"), @@ -367,8 +367,8 @@ def __init__(self, item_code, var_name=None, attributes=None, self.cell_methods = [] self.data = None - # Mimic a coordinate dictionary keys for iris coordinate names. This - # ensures the access key for coord() matches the coordinate's name + # Mimic a coordinate dictionary with iris coordinate names as keys to + # ensure the coord() access key matches the coordinate's name self._coordinates = {c.name(): c for c in coords} if coords else {} section, item = um2nc.to_stash_code(item_code) @@ -419,7 +419,7 @@ def heaviside_t_cube(lat_river_coord, lon_river_coord): # cube filtering tests -# use wrap results in tuples to capture generator output in sequence +# NB: wrap results in tuples to capture generator output in sequences def test_cube_filtering_mutually_exclusive(ua_plev_cube, heaviside_uv_cube): include = [30201] @@ -992,6 +992,7 @@ def level_coords(level_heights): um2nc.SIGMA: iris.coords.AuxCoord(np.array([0.99771646]))} +# TODO: replace this with a DummyCube @pytest.fixture def get_fake_cube_coords(level_coords): From c53f11a16c9636f95af9f5c82f2e2e19252480d6 Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Thu, 26 Sep 2024 14:53:51 +1000 Subject: [PATCH 03/15] Refactor: remove setup included in DummyCube. --- test/test_um2netcdf.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index f51589d..699309e 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -200,13 +200,6 @@ def test_process_mask_with_heaviside(air_temp_cube, precipitation_flux_cube, cubes = [air_temp_cube, precipitation_flux_cube, geo_potential_cube, heaviside_uv_cube, heaviside_t_cube] - # TODO: convert heaviside cubes to NonCallableMagicMock like other fixtures? - for c in [heaviside_uv_cube, heaviside_t_cube]: - # add attrs to mimic real cubes - attrs = {um2nc.STASH: DummyStash(*um2nc.to_stash_code(c.item_code))} - c.attributes = attrs - c.cell_methods = [] - m_iris_load.return_value = cubes m_saver().__enter__ = mock.Mock(name="mock_sman") From d6c2207c0b063d6da1e74501ba30b2a8d027eb81 Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Thu, 26 Sep 2024 15:39:44 +1000 Subject: [PATCH 04/15] Refactor fix_pressure_levels() & fix_level_coord() tests with DummyCube. --- test/test_um2netcdf.py | 66 +++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index 699309e..59a6974 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -975,48 +975,31 @@ def test_fix_cell_methods_keep_weeks(): def level_heights(): # NB: sourced from z_sea_theta_data fixture. This "array" is cropped as # fix_level_coords() only accesses height array[0] - return [20.0003377] + return [20.0003377] # TODO: add points to make data slightly more realistic? @pytest.fixture def level_coords(level_heights): - return {um2nc.MODEL_LEVEL_NUM: iris.coords.DimCoord(range(1, 39)), - um2nc.LEVEL_HEIGHT: iris.coords.DimCoord(level_heights), - um2nc.SIGMA: iris.coords.AuxCoord(np.array([0.99771646]))} + return [iris.coords.DimCoord(range(1, 39), var_name=um2nc.MODEL_LEVEL_NUM), + iris.coords.DimCoord(level_heights, var_name=um2nc.LEVEL_HEIGHT), + iris.coords.AuxCoord(np.array([0.99771646]), var_name=um2nc.SIGMA)] -# TODO: replace this with a DummyCube @pytest.fixture -def get_fake_cube_coords(level_coords): +def level_coords_cube(level_coords): + return DummyCube(0, coords=level_coords) - @dataclass - class FakeCubeCoords: - """Test object to represent a cube with a coords() access function.""" - def __init__(self, custom_coord: dict = None): - if custom_coord: - level_coords.update(custom_coord) - - def coord(self, key): - if key in level_coords: - return level_coords[key] - - msg = f"{self.__class__}: lacks coord for '{key}'" - raise iris.exceptions.CoordinateNotFoundError(msg) - - # return class for instantiation in tests - return FakeCubeCoords - - -def test_fix_level_coord_modify_cube_with_rho(level_heights, - get_fake_cube_coords, +def test_fix_level_coord_modify_cube_with_rho(level_coords_cube, + level_heights, z_sea_rho_data, z_sea_theta_data): # verify cube renaming with appropriate z_rho data - cube = get_fake_cube_coords() - assert cube.coord(um2nc.MODEL_LEVEL_NUM).var_name is None - assert cube.coord(um2nc.LEVEL_HEIGHT).var_name is None - assert cube.coord(um2nc.SIGMA).var_name is None + cube = level_coords_cube + + assert cube.coord(um2nc.MODEL_LEVEL_NUM).var_name != "model_rho_level_number" + assert cube.coord(um2nc.LEVEL_HEIGHT).var_name != "rho_level_height" + assert cube.coord(um2nc.SIGMA).var_name != "sigma_rho" rho = np.ones(z_sea_theta_data.shape) * level_heights[0] um2nc.fix_level_coord(cube, rho, z_sea_theta_data) @@ -1027,11 +1010,11 @@ def test_fix_level_coord_modify_cube_with_rho(level_heights, def test_fix_level_coord_modify_cube_with_theta(level_heights, - get_fake_cube_coords, + level_coords_cube, z_sea_rho_data, z_sea_theta_data): # verify cube renaming with appropriate z_theta data - cube = get_fake_cube_coords() + cube = level_coords_cube um2nc.fix_level_coord(cube, z_sea_rho_data, z_sea_theta_data) assert cube.coord(um2nc.MODEL_LEVEL_NUM).var_name == "model_theta_level_number" @@ -1048,8 +1031,8 @@ def test_fix_level_coord_skipped_if_no_levels(z_sea_rho_data, z_sea_theta_data): # tests - fix pressure level data -def test_fix_pressure_levels_no_pressure_coord(get_fake_cube_coords): - cube = get_fake_cube_coords() +def test_fix_pressure_levels_no_pressure_coord(level_coords_cube): + cube = level_coords_cube with pytest.raises(iris.exceptions.CoordinateNotFoundError): cube.coord("pressure") # ensure missing 'pressure' coord @@ -1065,11 +1048,14 @@ def _add_attrs_points(m_plevs: mock.MagicMock, points): setattr(m_plevs, "points", points) -def test_fix_pressure_levels_do_rounding(get_fake_cube_coords): - m_pressure = mock.Mock() - _add_attrs_points(m_pressure, [1.000001, 0.000001]) - extra = {"pressure": m_pressure} - cube = get_fake_cube_coords(extra) +def test_fix_pressure_levels_do_rounding(): + # init pressure coordinate + pressure = iris.coords.DimCoord([1.000001, 0.000001], + var_name="pressure", + units="Pa", + attributes={"positive": None}) + + cube = DummyCube(1, coords=[pressure]) # ensure no cube is returned if Cube not modified in fix_pressure_levels() assert um2nc.fix_pressure_levels(cube) is None @@ -1080,7 +1066,7 @@ def test_fix_pressure_levels_do_rounding(get_fake_cube_coords): @pytest.mark.skip -def test_fix_pressure_levels_reverse_pressure(get_fake_cube_coords): +def test_fix_pressure_levels_reverse_pressure(level_coords_cube): # TODO: test is broken due to fiddly mocking problems (see below) m_pressure = mock.Mock() From e3998adf8b1d8b01164801fd35423f7312dd8954 Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Thu, 26 Sep 2024 16:01:13 +1000 Subject: [PATCH 05/15] Remove old pressure coord setup. Tweak skipped pressure test with DummyCube for future implementation. --- test/test_um2netcdf.py | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index 59a6974..f6255a1 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -1041,15 +1041,7 @@ def test_fix_pressure_levels_no_pressure_coord(level_coords_cube): assert um2nc.fix_pressure_levels(cube) is None # should just exit -def _add_attrs_points(m_plevs: mock.MagicMock, points): - # NB: iris attributes appear to be added via mixins, so it's easier but - # less desirable to rely on mock attrs here - setattr(m_plevs, "attributes", {"positive": None}) - setattr(m_plevs, "points", points) - - def test_fix_pressure_levels_do_rounding(): - # init pressure coordinate pressure = iris.coords.DimCoord([1.000001, 0.000001], var_name="pressure", units="Pa", @@ -1066,15 +1058,15 @@ def test_fix_pressure_levels_do_rounding(): @pytest.mark.skip -def test_fix_pressure_levels_reverse_pressure(level_coords_cube): +def test_fix_pressure_levels_reverse_pressure(): # TODO: test is broken due to fiddly mocking problems (see below) + pressure = iris.coords.DimCoord([0.000001, 1.000001], + var_name="pressure", + units="Pa", + attributes={"positive": None}) - m_pressure = mock.Mock() - # m_pressure.ndim = 1 - _add_attrs_points(m_pressure, [0.000001, 1.000001]) - extra = {"pressure": m_pressure} - cube = get_fake_cube_coords(extra) - # cube.ndim = 3 + cube = DummyCube(1, coords=[pressure]) + cube.ndim = 3 # TODO: testing gets odd here at the um2nc & iris "boundary": # * A mock reverse() needs to flip pressure.points & return a modified cube. @@ -1090,8 +1082,10 @@ def test_fix_pressure_levels_reverse_pressure(level_coords_cube): # # The test is disabled awaiting a solution... - with mock.patch("iris.util.reverse"): - mod_cube = um2nc.fix_pressure_levels(cube) + # with mock.patch("iris.util.reverse"): + # mod_cube = um2nc.fix_pressure_levels(cube) + + mod_cube = um2nc.fix_pressure_levels(cube) # breaks on missing __getitem__ assert mod_cube is not None assert mod_cube != cube From 06b23c0fe84fbf965a02c21d88d33fcd6d759415 Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Fri, 27 Sep 2024 16:28:55 +1000 Subject: [PATCH 06/15] Minor comment fixes. --- test/test_um2netcdf.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index f6255a1..d1ffc00 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -123,8 +123,7 @@ def fake_out_path(): # FIXME: the convoluted setup in test_process_...() is a code smell # use the following tests to gradually refactor process() # TODO: evolve towards design where input & output file I/O is extracted from -# process() & the function only takes *raw data only* (is highly testable) - +# process() & the function takes *raw data only* (is highly testable) def test_process_no_heaviside_drop_cubes(air_temp_cube, precipitation_flux_cube, geo_potential_cube, mule_vars, std_args, fake_in_path, fake_out_path): @@ -446,7 +445,7 @@ def test_cube_filtering_no_include_exclude(ua_plev_cube, heaviside_uv_cube): @pytest.fixture def x_wind_cube(): x_wind_cube = DummyCube(2, var_name="var_name", - attributes={'STASH': DummyStash(0, 2)}) + attributes={'STASH': DummyStash(0, 2)}) x_wind_cube.standard_name = "x_wind" x_wind_cube.cell_methods = [] return x_wind_cube @@ -980,6 +979,7 @@ def level_heights(): @pytest.fixture def level_coords(level_heights): + # data likely extracted from aiihca.subset return [iris.coords.DimCoord(range(1, 39), var_name=um2nc.MODEL_LEVEL_NUM), iris.coords.DimCoord(level_heights, var_name=um2nc.LEVEL_HEIGHT), iris.coords.AuxCoord(np.array([0.99771646]), var_name=um2nc.SIGMA)] From df1354d4746321eba49d47b121474b939993d3b7 Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Fri, 27 Sep 2024 16:34:34 +1000 Subject: [PATCH 07/15] Refactor some repeated strings to constants. --- test/test_um2netcdf.py | 18 +++++++++--------- umpost/um2netcdf.py | 8 ++++++++ 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index d1ffc00..84886b8 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -997,16 +997,16 @@ def test_fix_level_coord_modify_cube_with_rho(level_coords_cube, # verify cube renaming with appropriate z_rho data cube = level_coords_cube - assert cube.coord(um2nc.MODEL_LEVEL_NUM).var_name != "model_rho_level_number" - assert cube.coord(um2nc.LEVEL_HEIGHT).var_name != "rho_level_height" - assert cube.coord(um2nc.SIGMA).var_name != "sigma_rho" + assert cube.coord(um2nc.MODEL_LEVEL_NUM).var_name != um2nc.MODEL_RHO_LEVEL + assert cube.coord(um2nc.LEVEL_HEIGHT).var_name != um2nc.RHO_LEVEL_HEIGHT + assert cube.coord(um2nc.SIGMA).var_name != um2nc.SIGMA_RHO rho = np.ones(z_sea_theta_data.shape) * level_heights[0] um2nc.fix_level_coord(cube, rho, z_sea_theta_data) - assert cube.coord(um2nc.MODEL_LEVEL_NUM).var_name == "model_rho_level_number" - assert cube.coord(um2nc.LEVEL_HEIGHT).var_name == "rho_level_height" - assert cube.coord(um2nc.SIGMA).var_name == "sigma_rho" + assert cube.coord(um2nc.MODEL_LEVEL_NUM).var_name == um2nc.MODEL_RHO_LEVEL + assert cube.coord(um2nc.LEVEL_HEIGHT).var_name == um2nc.RHO_LEVEL_HEIGHT + assert cube.coord(um2nc.SIGMA).var_name == um2nc.SIGMA_RHO def test_fix_level_coord_modify_cube_with_theta(level_heights, @@ -1017,9 +1017,9 @@ def test_fix_level_coord_modify_cube_with_theta(level_heights, cube = level_coords_cube um2nc.fix_level_coord(cube, z_sea_rho_data, z_sea_theta_data) - assert cube.coord(um2nc.MODEL_LEVEL_NUM).var_name == "model_theta_level_number" - assert cube.coord(um2nc.LEVEL_HEIGHT).var_name == "theta_level_height" - assert cube.coord(um2nc.SIGMA).var_name == "sigma_theta" + assert cube.coord(um2nc.MODEL_LEVEL_NUM).var_name == um2nc.MODEL_THETA_LEVEL_NUM + assert cube.coord(um2nc.LEVEL_HEIGHT).var_name == um2nc.THETA_LEVEL_HEIGHT + assert cube.coord(um2nc.SIGMA).var_name == um2nc.SIGMA_THETA def test_fix_level_coord_skipped_if_no_levels(z_sea_rho_data, z_sea_theta_data): diff --git a/umpost/um2netcdf.py b/umpost/um2netcdf.py index de886d3..147e98b 100644 --- a/umpost/um2netcdf.py +++ b/umpost/um2netcdf.py @@ -67,8 +67,16 @@ } MODEL_LEVEL_NUM = "model_level_number" +MODEL_RHO_LEVEL = "model_rho_level_number" +MODEL_THETA_LEVEL_NUM = "model_theta_level_number" + LEVEL_HEIGHT = "level_height" +THETA_LEVEL_HEIGHT = "theta_level_height" +RHO_LEVEL_HEIGHT = "rho_level_height" + SIGMA = "sigma" +SIGMA_THETA = "sigma_theta" +SIGMA_RHO = "sigma_rho" class PostProcessingError(Exception): From fe72fa8179ea99b6f0065260327421df60f44b71 Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Mon, 30 Sep 2024 19:29:21 +1000 Subject: [PATCH 08/15] Reorder set item tests. --- test/test_um2netcdf.py | 49 +++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index 84886b8..672d0d3 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -314,20 +314,6 @@ def test_stash_code_to_item_code_conversion(): assert result == 30255 -@dataclass(frozen=True) -class DummyStash: - """ - Partial Stash representation for testing. - """ - section: int - item: int - - -def add_stash(cube, stash): - d = {um2nc.STASH: stash} - setattr(cube, "attributes", d) - - def test_set_item_codes(): cube0 = DummyCube(1002, "d0", {um2nc.STASH: DummyStash(1, 2)}) cube1 = DummyCube(3004, "d1", {um2nc.STASH: DummyStash(3, 4)}) @@ -343,6 +329,31 @@ def test_set_item_codes(): assert c1.item_code == 3004 +def test_set_item_codes_avoid_overwrite(): + item_code = 1007 + item_code2 = 51006 + + cubes = [DummyCube(item_code, "fake_var"), DummyCube(item_code2, "fake_var2")] + um2nc.set_item_codes(cubes) + assert cubes[0].item_code == item_code + assert cubes[1].item_code == item_code2 + + +@dataclass(frozen=True) +class DummyStash: + """ + Partial Stash representation for testing. + """ + section: int + item: int + + +def add_stash(cube, stash): + d = {um2nc.STASH: stash} + setattr(cube, "attributes", d) + + + class DummyCube: """ Imitation iris Cube for unit testing. @@ -377,16 +388,6 @@ def coord(self, _name): raise CoordinateNotFoundError(msg) -def test_set_item_codes_avoid_overwrite(): - item_code = 1007 - item_code2 = 51006 - - cubes = [DummyCube(item_code, "fake_var"), DummyCube(item_code2, "fake_var2")] - um2nc.set_item_codes(cubes) - assert cubes[0].item_code == item_code - assert cubes[1].item_code == item_code2 - - @pytest.fixture def ua_plev_cube(): return DummyCube(30201, "ua_plev") From f7fe5d94f6c44b9430ece07b17db81f4e5448a75 Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Mon, 30 Sep 2024 19:42:56 +1000 Subject: [PATCH 09/15] Cleanup text fixtures now using DummyCube. --- test/test_um2netcdf.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index 672d0d3..163d525 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -445,10 +445,8 @@ def test_cube_filtering_no_include_exclude(ua_plev_cube, heaviside_uv_cube): # cube variable renaming tests @pytest.fixture def x_wind_cube(): - x_wind_cube = DummyCube(2, var_name="var_name", - attributes={'STASH': DummyStash(0, 2)}) + x_wind_cube = DummyCube(2, var_name="var_name") x_wind_cube.standard_name = "x_wind" - x_wind_cube.cell_methods = [] return x_wind_cube @@ -510,9 +508,8 @@ def test_fix_standard_name_update_x_wind(x_wind_cube): def test_fix_standard_name_update_y_wind(): # test cube wind renaming block only # use empty std name to bypass renaming logic - m_cube = DummyCube(3, attributes={'STASH': DummyStash(0, 3)}) + m_cube = DummyCube(3) m_cube.standard_name = "y_wind" - m_cube.cell_methods = [] um2nc.fix_standard_name(m_cube, "", verbose=False) assert m_cube.standard_name == "northward_wind" From 3a774ed2a6da8791e5d19f5298b48e4eaf7dab7d Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Tue, 1 Oct 2024 11:05:21 +1000 Subject: [PATCH 10/15] Use correct coordinate fixtures for cube test fixtures. --- test/test_um2netcdf.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index 163d525..0497391 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -79,18 +79,18 @@ def air_temp_cube(lat_river_coord, lon_river_coord): @pytest.fixture -def precipitation_flux_cube(lat_river_coord, lon_river_coord): +def precipitation_flux_cube(lat_standard_nd_coord, lon_standard_nd_coord): # copied from aiihca.paa1jan.subset file precipitation_flux = DummyCube(5216, "precipitation_flux", - coords=[lat_river_coord, lon_river_coord]) + coords=[lat_standard_nd_coord, lon_standard_nd_coord]) return precipitation_flux @pytest.fixture -def geo_potential_cube(lat_river_coord, lon_river_coord): +def geo_potential_cube(lat_v_nd_coord, lon_u_nd_coord): """Return new cube requiring heaviside_t masking""" - geo_potential = DummyCube(30297, "geopotential_height", - coords=[lat_river_coord, lon_river_coord]) + geo_potential = DummyCube(30207, "geopotential_height", + coords=[lat_v_nd_coord, lon_u_nd_coord]) return geo_potential @@ -394,21 +394,21 @@ def ua_plev_cube(): @pytest.fixture -def heaviside_uv_cube(lat_river_coord, lon_river_coord): +def heaviside_uv_cube(lat_v_nd_coord, lon_u_nd_coord): return DummyCube(30301, "heaviside_uv", - coords=[lat_river_coord, lon_river_coord]) + coords=[lat_v_nd_coord, lon_u_nd_coord]) @pytest.fixture -def ta_plev_cube(lat_river_coord, lon_river_coord): - return DummyCube(30294, "ta_plev", - coords=[lat_river_coord, lon_river_coord]) +def ta_plev_cube(lat_v_nd_coord, lon_u_nd_coord): + return DummyCube(30204, "ta_plev", + coords=[lat_v_nd_coord, lon_u_nd_coord]) @pytest.fixture -def heaviside_t_cube(lat_river_coord, lon_river_coord): +def heaviside_t_cube(lat_standard_eg_coord, lon_standard_eg_coord): return DummyCube(30304, "heaviside_t", - coords=[lat_river_coord, lon_river_coord]) + coords=[lat_standard_eg_coord, lon_standard_eg_coord]) # cube filtering tests From 8cf02c84f72f41e6340c269c8274541b88d8dbaa Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Tue, 1 Oct 2024 11:08:00 +1000 Subject: [PATCH 11/15] Replace duplicate air_temp_cube fixture with ta_plev_cube. --- test/test_um2netcdf.py | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index 0497391..c009134 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -70,14 +70,6 @@ def mule_vars(z_sea_rho_data, z_sea_theta_data): return um2nc.MuleVars(um2nc.GRID_NEW_DYNAMICS, d_lat, d_lon, z_sea_rho_data, z_sea_theta_data) -@pytest.fixture -def air_temp_cube(lat_river_coord, lon_river_coord): - # details copied from aiihca.paa1jan.subset file - air_temp = DummyCube(30204, "air_temperature", - coords=[lat_river_coord, lon_river_coord]) - return air_temp - - @pytest.fixture def precipitation_flux_cube(lat_standard_nd_coord, lon_standard_nd_coord): # copied from aiihca.paa1jan.subset file @@ -124,7 +116,7 @@ def fake_out_path(): # use the following tests to gradually refactor process() # TODO: evolve towards design where input & output file I/O is extracted from # process() & the function takes *raw data only* (is highly testable) -def test_process_no_heaviside_drop_cubes(air_temp_cube, precipitation_flux_cube, +def test_process_no_heaviside_drop_cubes(ta_plev_cube, precipitation_flux_cube, geo_potential_cube, mule_vars, std_args, fake_in_path, fake_out_path): """Attempt end-to-end process() test, dropping cubes requiring masking.""" @@ -140,7 +132,7 @@ def test_process_no_heaviside_drop_cubes(air_temp_cube, precipitation_flux_cube, # include cubes requiring both heaviside uv & t cubes to filter, to # ensure both uv/t dependent cubes are dropped - cubes = [air_temp_cube, precipitation_flux_cube, geo_potential_cube] + cubes = [ta_plev_cube, precipitation_flux_cube, geo_potential_cube] m_iris_load.return_value = cubes m_saver().__enter__ = mock.Mock(name="mock_sman") @@ -161,7 +153,7 @@ def test_process_no_heaviside_drop_cubes(air_temp_cube, precipitation_flux_cube, assert cube.data is None # masking wasn't called/nothing changed -def test_process_all_cubes_filtered(air_temp_cube, geo_potential_cube, +def test_process_all_cubes_filtered(ta_plev_cube, geo_potential_cube, mule_vars, std_args, fake_in_path, fake_out_path): """Ensure process() exits early if all cubes are removed in filtering.""" @@ -172,14 +164,14 @@ def test_process_all_cubes_filtered(air_temp_cube, geo_potential_cube, mock.patch("iris.fileformats.netcdf.Saver") as m_saver, # prevent I/O ): m_mule_vars.return_value = mule_vars - m_iris_load.return_value = [air_temp_cube, geo_potential_cube] + m_iris_load.return_value = [ta_plev_cube, geo_potential_cube] m_saver().__enter__ = mock.Mock(name="mock_sman") # all cubes should be dropped assert um2nc.process(fake_in_path, fake_out_path, std_args) == [] -def test_process_mask_with_heaviside(air_temp_cube, precipitation_flux_cube, +def test_process_mask_with_heaviside(ta_plev_cube, precipitation_flux_cube, heaviside_uv_cube, heaviside_t_cube, geo_potential_cube, mule_vars, std_args, fake_in_path, fake_out_path): @@ -196,7 +188,7 @@ def test_process_mask_with_heaviside(air_temp_cube, precipitation_flux_cube, # air temp requires heaviside_uv & geo_potential_cube requires heaviside_t # masking, include both to enable code execution for both masks - cubes = [air_temp_cube, precipitation_flux_cube, geo_potential_cube, + cubes = [ta_plev_cube, precipitation_flux_cube, geo_potential_cube, heaviside_uv_cube, heaviside_t_cube] m_iris_load.return_value = cubes @@ -210,7 +202,7 @@ def test_process_mask_with_heaviside(air_temp_cube, precipitation_flux_cube, assert pc in cubes -def test_process_no_masking_keep_all_cubes(air_temp_cube, precipitation_flux_cube, +def test_process_no_masking_keep_all_cubes(ta_plev_cube, precipitation_flux_cube, geo_potential_cube, mule_vars, std_args, fake_in_path, fake_out_path): """Run process() with masking off, ensuring all cubes are kept & modified.""" @@ -224,7 +216,7 @@ def test_process_no_masking_keep_all_cubes(air_temp_cube, precipitation_flux_cub m_mule_vars.return_value = mule_vars # air temp and geo potential would need heaviside uv & t respectively - cubes = [air_temp_cube, precipitation_flux_cube, geo_potential_cube] + cubes = [ta_plev_cube, precipitation_flux_cube, geo_potential_cube] m_iris_load.return_value = cubes m_saver().__enter__ = mock.Mock(name="mock_sman") @@ -353,7 +345,6 @@ def add_stash(cube, stash): setattr(cube, "attributes", d) - class DummyCube: """ Imitation iris Cube for unit testing. From e4ca35cf0e2f526922738e89cd3b56d38248e7b6 Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Tue, 1 Oct 2024 11:12:04 +1000 Subject: [PATCH 12/15] Move general use cube fixtures together. --- test/test_um2netcdf.py | 46 +++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index c009134..2d65999 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -86,6 +86,29 @@ def geo_potential_cube(lat_v_nd_coord, lon_u_nd_coord): return geo_potential +@pytest.fixture +def ua_plev_cube(): + return DummyCube(30201, "ua_plev") + + +@pytest.fixture +def heaviside_uv_cube(lat_v_nd_coord, lon_u_nd_coord): + return DummyCube(30301, "heaviside_uv", + coords=[lat_v_nd_coord, lon_u_nd_coord]) + + +@pytest.fixture +def ta_plev_cube(lat_v_nd_coord, lon_u_nd_coord): + return DummyCube(30204, "ta_plev", + coords=[lat_v_nd_coord, lon_u_nd_coord]) + + +@pytest.fixture +def heaviside_t_cube(lat_standard_eg_coord, lon_standard_eg_coord): + return DummyCube(30304, "heaviside_t", + coords=[lat_standard_eg_coord, lon_standard_eg_coord]) + + @pytest.fixture def std_args(): # TODO: make args namedtuple? @@ -379,29 +402,6 @@ def coord(self, _name): raise CoordinateNotFoundError(msg) -@pytest.fixture -def ua_plev_cube(): - return DummyCube(30201, "ua_plev") - - -@pytest.fixture -def heaviside_uv_cube(lat_v_nd_coord, lon_u_nd_coord): - return DummyCube(30301, "heaviside_uv", - coords=[lat_v_nd_coord, lon_u_nd_coord]) - - -@pytest.fixture -def ta_plev_cube(lat_v_nd_coord, lon_u_nd_coord): - return DummyCube(30204, "ta_plev", - coords=[lat_v_nd_coord, lon_u_nd_coord]) - - -@pytest.fixture -def heaviside_t_cube(lat_standard_eg_coord, lon_standard_eg_coord): - return DummyCube(30304, "heaviside_t", - coords=[lat_standard_eg_coord, lon_standard_eg_coord]) - - # cube filtering tests # NB: wrap results in tuples to capture generator output in sequences From 223c50b5d738d5f455d0e16a9592f8fda81d678f Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Tue, 1 Oct 2024 11:16:59 +1000 Subject: [PATCH 13/15] Move DummyCube & DummyStash test fixtures near top of module. --- test/test_um2netcdf.py | 86 +++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index 2d65999..3074187 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -70,6 +70,49 @@ def mule_vars(z_sea_rho_data, z_sea_theta_data): return um2nc.MuleVars(um2nc.GRID_NEW_DYNAMICS, d_lat, d_lon, z_sea_rho_data, z_sea_theta_data) +@dataclass(frozen=True) +class DummyStash: + """ + Partial Stash representation for testing. + """ + section: int + item: int + + +class DummyCube: + """ + Imitation iris Cube for unit testing. + """ + + def __init__(self, item_code, var_name=None, attributes=None, + units=None, coords=None): + self.item_code = item_code + self.var_name = var_name or "unknown_var" + self.attributes = attributes + self.units = units + self.standard_name = None + self.long_name = "" + self.cell_methods = [] + self.data = None + + # Mimic a coordinate dictionary with iris coordinate names as keys to + # ensure the coord() access key matches the coordinate's name + self._coordinates = {c.name(): c for c in coords} if coords else {} + + section, item = um2nc.to_stash_code(item_code) + self.attributes = {um2nc.STASH: DummyStash(section, item)} + + def name(self): + return self.var_name + + def coord(self, _name): + try: + return self._coordinates[_name] + except KeyError: + msg = f"{self.__class__}[{self.var_name}]: lacks coord for '{_name}'" + raise CoordinateNotFoundError(msg) + + @pytest.fixture def precipitation_flux_cube(lat_standard_nd_coord, lon_standard_nd_coord): # copied from aiihca.paa1jan.subset file @@ -354,54 +397,11 @@ def test_set_item_codes_avoid_overwrite(): assert cubes[1].item_code == item_code2 -@dataclass(frozen=True) -class DummyStash: - """ - Partial Stash representation for testing. - """ - section: int - item: int - - def add_stash(cube, stash): d = {um2nc.STASH: stash} setattr(cube, "attributes", d) -class DummyCube: - """ - Imitation iris Cube for unit testing. - """ - - def __init__(self, item_code, var_name=None, attributes=None, - units=None, coords=None): - self.item_code = item_code - self.var_name = var_name or "unknown_var" - self.attributes = attributes - self.units = units - self.standard_name = None - self.long_name = "" - self.cell_methods = [] - self.data = None - - # Mimic a coordinate dictionary with iris coordinate names as keys to - # ensure the coord() access key matches the coordinate's name - self._coordinates = {c.name(): c for c in coords} if coords else {} - - section, item = um2nc.to_stash_code(item_code) - self.attributes = {um2nc.STASH: DummyStash(section, item)} - - def name(self): - return self.var_name - - def coord(self, _name): - try: - return self._coordinates[_name] - except KeyError: - msg = f"{self.__class__}[{self.var_name}]: lacks coord for '{_name}'" - raise CoordinateNotFoundError(msg) - - # cube filtering tests # NB: wrap results in tuples to capture generator output in sequences From a84ba48f45a31c245a6ee44c8eea2c5026148a69 Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Wed, 2 Oct 2024 12:23:20 +1000 Subject: [PATCH 14/15] Revert geo_potential cube item code & add comments. --- test/test_um2netcdf.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index 3074187..2b7cd52 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -113,6 +113,11 @@ def coord(self, _name): raise CoordinateNotFoundError(msg) +# NB: these cube fixtures have been chosen to mimic cubes for testing key parts +# of the process() workflow. Some cubes require pressure level masking with the +# heaviside_uv/t cubes. These cubes facilitate different testing configurations. +# Modifying them has the potential to reduce test coverage! + @pytest.fixture def precipitation_flux_cube(lat_standard_nd_coord, lon_standard_nd_coord): # copied from aiihca.paa1jan.subset file @@ -124,7 +129,7 @@ def precipitation_flux_cube(lat_standard_nd_coord, lon_standard_nd_coord): @pytest.fixture def geo_potential_cube(lat_v_nd_coord, lon_u_nd_coord): """Return new cube requiring heaviside_t masking""" - geo_potential = DummyCube(30207, "geopotential_height", + geo_potential = DummyCube(30297, "geopotential_height", coords=[lat_v_nd_coord, lon_u_nd_coord]) return geo_potential From 8b244b127ccceccc5662353fc2f3d8bc9c80233b Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Wed, 2 Oct 2024 14:25:30 +1000 Subject: [PATCH 15/15] Use correct coords for geo_potential_cube fixture. --- test/test_um2netcdf.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py index 2b7cd52..7aedce3 100644 --- a/test/test_um2netcdf.py +++ b/test/test_um2netcdf.py @@ -127,10 +127,10 @@ def precipitation_flux_cube(lat_standard_nd_coord, lon_standard_nd_coord): @pytest.fixture -def geo_potential_cube(lat_v_nd_coord, lon_u_nd_coord): +def geo_potential_cube(lat_standard_eg_coord, lon_standard_eg_coord): """Return new cube requiring heaviside_t masking""" geo_potential = DummyCube(30297, "geopotential_height", - coords=[lat_v_nd_coord, lon_u_nd_coord]) + coords=[lat_standard_eg_coord, lon_standard_eg_coord]) return geo_potential