From 29c29adc6608b7ede33da5f5a372983a835ac057 Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Thu, 29 Aug 2024 16:03:45 +1000
Subject: [PATCH 01/17] Move cubewrite() plevs code to fix function.

---
 umpost/um2netcdf.py | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/umpost/um2netcdf.py b/umpost/um2netcdf.py
index 80f55ef..40ef702 100644
--- a/umpost/um2netcdf.py
+++ b/umpost/um2netcdf.py
@@ -147,17 +147,7 @@ def _add_coord_bounds(coord):
 
 # TODO: split cube ops into functions, this will likely increase process() workflow steps
 def cubewrite(cube, sman, compression, use64bit, verbose):
-    try:
-        plevs = cube.coord('pressure')
-        plevs.attributes['positive'] = 'down'
-        plevs.convert_units('Pa')
-        # Otherwise they're off by 1e-10 which looks odd in ncdump
-        plevs.points = np.round(plevs.points, 5)
-        if plevs.points[0] < plevs.points[-1]:
-            # Flip to get pressure decreasing as in CMIP6 standard
-            cube = iris.util.reverse(cube, 'pressure')
-    except iris.exceptions.CoordinateNotFoundError:
-        pass
+    fix_plevs(cube)
 
     if not use64bit:
         if cube.data.dtype == 'float64':
@@ -736,6 +726,27 @@ def fix_level_coord(cube, z_rho, z_theta, tol=1e-6):
                 c_sigma.var_name = 'sigma_theta'
 
 
+def fix_plevs(cube):
+    """
+    TODO
+
+    Parameters
+    ----------
+    cube : iris Cube (modifies in place)
+    """
+    try:
+        plevs = cube.coord('pressure')
+        plevs.attributes['positive'] = 'down'
+        plevs.convert_units('Pa')
+        # Otherwise they're off by 1e-10 which looks odd in ncdump
+        plevs.points = np.round(plevs.points, 5)
+        if plevs.points[0] < plevs.points[-1]:
+            # Flip to get pressure decreasing as in CMIP6 standard
+            cube = iris.util.reverse(cube, 'pressure')
+    except iris.exceptions.CoordinateNotFoundError:
+        pass
+
+
 def parse_args():
     parser = argparse.ArgumentParser(description="Convert UM fieldsfile to netcdf")
     parser.add_argument('-k', dest='nckind', required=False, type=int,

From 4ae059aaaed92064101970815a4e89ad0e59d8c9 Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Thu, 29 Aug 2024 17:07:30 +1000
Subject: [PATCH 02/17] Update FakeCubeCoords to allow custom coordinates.

---
 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 c1a86f0..4f9de86 100644
--- a/test/test_um2netcdf.py
+++ b/test/test_um2netcdf.py
@@ -683,12 +683,14 @@ def level_heights():
     #     fix_level_coords() only accesses height array[0]
     return [20.0003377]
 
+
 @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]))}
 
+
 @pytest.fixture
 def get_fake_cube_coords(level_coords):
 
@@ -696,6 +698,10 @@ def get_fake_cube_coords(level_coords):
     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):
             return level_coords[key]
 
@@ -713,7 +719,6 @@ def test_fix_level_coord_modify_cube_with_rho(level_heights,
     assert cube.coord(um2nc.LEVEL_HEIGHT).var_name is None
     assert cube.coord(um2nc.SIGMA).var_name is None
 
-
     rho = np.ones(z_sea_theta_data.shape) * level_heights[0]
     um2nc.fix_level_coord(cube, rho, z_sea_theta_data)
 

From 7bc9b6ea7e53907f12b8862b1d74a64e646fc460 Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Thu, 29 Aug 2024 17:08:04 +1000
Subject: [PATCH 03/17] Add draft pressure level tests.

---
 test/test_um2netcdf.py | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py
index 4f9de86..493824b 100644
--- a/test/test_um2netcdf.py
+++ b/test/test_um2netcdf.py
@@ -745,3 +745,44 @@ def test_fix_level_coord_skipped_if_no_levels(z_sea_rho_data, z_sea_theta_data):
     m_cube = mock.Mock(iris.cube.Cube)
     m_cube.coord.side_effect = iris.exceptions.CoordinateNotFoundError
     um2nc.fix_level_coord(m_cube, z_sea_rho_data, z_sea_theta_data)
+
+
+# fix pressure level tests
+
+def test_fix_plevs_no_pressure_coord(get_fake_cube_coords):
+    cube = get_fake_cube_coords()
+
+    with pytest.raises(KeyError):
+        um2nc.fix_plevs(cube)  # ensure there's no 'pressure' key
+
+
+def _add_attrs_points(m_plevs: mock.MagicMock, points):
+    setattr(m_plevs, "attributes", {"positive": None})
+    setattr(m_plevs, "points", points)
+
+
+def test_fix_plevs_do_rounding(get_fake_cube_coords):
+    m_plevs = mock.Mock()
+    _add_attrs_points(m_plevs, [1.000001, 0.000001])
+    extra = {"pressure": m_plevs}
+    cube = get_fake_cube_coords(extra)
+
+    um2nc.fix_plevs(cube)
+
+    plev = cube.coord('pressure')
+    assert plev.attributes["positive"] == "down"
+    assert all(plev.points == [1.0, 0.0])
+
+
+def test_fix_plevs_reverse_pressure(get_fake_cube_coords):
+    m_plevs = mock.Mock()
+    _add_attrs_points(m_plevs, [0.000001, 1.000001])
+    extra = {"pressure": m_plevs}
+    cube = get_fake_cube_coords(extra)
+
+    with mock.patch("iris.util.reverse"):
+        um2nc.fix_plevs(cube)
+
+    plev = cube.coord('pressure')
+    assert plev.attributes["positive"] == "down"
+    assert all(plev.points == [0.0, 1.0])

From cd5b00ec71c1c23729d8c30f008da03a03778d8e Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Fri, 30 Aug 2024 11:18:43 +1000
Subject: [PATCH 04/17] Fix FakeCubeCoords to raise errors on missing
 coordinate.

Update test.
---
 test/test_um2netcdf.py | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py
index 493824b..7dffd4e 100644
--- a/test/test_um2netcdf.py
+++ b/test/test_um2netcdf.py
@@ -703,7 +703,11 @@ def __init__(self, custom_coord: dict = None):
                 level_coords.update(custom_coord)
 
         def coord(self, key):
-            return level_coords[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
@@ -747,16 +751,20 @@ def test_fix_level_coord_skipped_if_no_levels(z_sea_rho_data, z_sea_theta_data):
     um2nc.fix_level_coord(m_cube, z_sea_rho_data, z_sea_theta_data)
 
 
-# fix pressure level tests
+# tests - fix pressure level data
 
 def test_fix_plevs_no_pressure_coord(get_fake_cube_coords):
     cube = get_fake_cube_coords()
 
-    with pytest.raises(KeyError):
-        um2nc.fix_plevs(cube)  # ensure there's no 'pressure' key
+    with pytest.raises(iris.exceptions.CoordinateNotFoundError):
+        cube.coord("pressure")  # ensure missing 'pressure' coord
+
+    um2nc.fix_plevs(cube)  # 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)
 

From df2e8f364e4ffd59e485ddd22e660bdbcb49e8aa Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Fri, 30 Aug 2024 12:20:10 +1000
Subject: [PATCH 05/17] Refactor fix_plevs() & add warning comments.

---
 test/test_um2netcdf.py |  6 ++++++
 umpost/um2netcdf.py    | 34 ++++++++++++++++++++++++----------
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py
index 7dffd4e..3fee38b 100644
--- a/test/test_um2netcdf.py
+++ b/test/test_um2netcdf.py
@@ -777,6 +777,9 @@ def test_fix_plevs_do_rounding(get_fake_cube_coords):
 
     um2nc.fix_plevs(cube)
 
+    # TODO: test flaw, this verifies pressure coord but ignores fix_plevs()
+    #       returning a new cube if the pressure is reversed. This is verified
+    #       in command line testing though
     plev = cube.coord('pressure')
     assert plev.attributes["positive"] == "down"
     assert all(plev.points == [1.0, 0.0])
@@ -791,6 +794,9 @@ def test_fix_plevs_reverse_pressure(get_fake_cube_coords):
     with mock.patch("iris.util.reverse"):
         um2nc.fix_plevs(cube)
 
+    # TODO: test flaw, this verifies pressure coord but ignores fix_plevs()
+    #       returning a new cube if the pressure is reversed. This is verified
+    #       in command line testing though
     plev = cube.coord('pressure')
     assert plev.attributes["positive"] == "down"
     assert all(plev.points == [0.0, 1.0])
diff --git a/umpost/um2netcdf.py b/umpost/um2netcdf.py
index 40ef702..2164dca 100644
--- a/umpost/um2netcdf.py
+++ b/umpost/um2netcdf.py
@@ -147,7 +147,7 @@ def _add_coord_bounds(coord):
 
 # TODO: split cube ops into functions, this will likely increase process() workflow steps
 def cubewrite(cube, sman, compression, use64bit, verbose):
-    fix_plevs(cube)
+    cube = fix_plevs(cube) or cube  # NB: use new cube if pressure points are modified
 
     if not use64bit:
         if cube.data.dtype == 'float64':
@@ -728,23 +728,37 @@ def fix_level_coord(cube, z_rho, z_theta, tol=1e-6):
 
 def fix_plevs(cube):
     """
-    TODO
+    Reformat pressure level data for NetCDF output.
+
+    This converts units, rounds small fractional errors & ensures pressure is
+    decreasing (following the CMIP6 standard).
 
     Parameters
     ----------
     cube : iris Cube (modifies in place)
+
+    Returns
+    -------
+    None if cube lacks pressure coord or is modified in place, otherwise a new
+    cube if the pressure levels are reversed.
     """
+    # TODO: add rounding places arg
     try:
         plevs = cube.coord('pressure')
-        plevs.attributes['positive'] = 'down'
-        plevs.convert_units('Pa')
-        # Otherwise they're off by 1e-10 which looks odd in ncdump
-        plevs.points = np.round(plevs.points, 5)
-        if plevs.points[0] < plevs.points[-1]:
-            # Flip to get pressure decreasing as in CMIP6 standard
-            cube = iris.util.reverse(cube, 'pressure')
     except iris.exceptions.CoordinateNotFoundError:
-        pass
+        return
+
+    # update existing cube metadata in place
+    plevs.attributes['positive'] = 'down'
+    plevs.convert_units('Pa')
+
+    # Round small fractions otherwise coordinates are off by 1e-10 in ncdump output
+    plevs.points = np.round(plevs.points, 5)
+
+    if plevs.points[0] < plevs.points[-1]:
+        # Flip to get pressure decreasing as per CMIP6 standard
+        # NOTE: returns a new cube!
+        return iris.util.reverse(cube, 'pressure')
 
 
 def parse_args():

From 07efb1a90418a7a29cbd124e6bc482c4cca4b8b2 Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Fri, 30 Aug 2024 12:24:35 +1000
Subject: [PATCH 06/17] Add number of decimals rounding arg to fix_plevs().

---
 umpost/um2netcdf.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/umpost/um2netcdf.py b/umpost/um2netcdf.py
index 2164dca..d840926 100644
--- a/umpost/um2netcdf.py
+++ b/umpost/um2netcdf.py
@@ -726,7 +726,7 @@ def fix_level_coord(cube, z_rho, z_theta, tol=1e-6):
                 c_sigma.var_name = 'sigma_theta'
 
 
-def fix_plevs(cube):
+def fix_plevs(cube, decimals=5):
     """
     Reformat pressure level data for NetCDF output.
 
@@ -736,13 +736,13 @@ def fix_plevs(cube):
     Parameters
     ----------
     cube : iris Cube (modifies in place)
+    decimals : number of decimals to round to
 
     Returns
     -------
     None if cube lacks pressure coord or is modified in place, otherwise a new
     cube if the pressure levels are reversed.
     """
-    # TODO: add rounding places arg
     try:
         plevs = cube.coord('pressure')
     except iris.exceptions.CoordinateNotFoundError:
@@ -753,7 +753,7 @@ def fix_plevs(cube):
     plevs.convert_units('Pa')
 
     # Round small fractions otherwise coordinates are off by 1e-10 in ncdump output
-    plevs.points = np.round(plevs.points, 5)
+    plevs.points = np.round(plevs.points, decimals)
 
     if plevs.points[0] < plevs.points[-1]:
         # Flip to get pressure decreasing as per CMIP6 standard

From 8c6abddc6ad0aab8e309b77b078548a205912adc Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Fri, 30 Aug 2024 12:29:31 +1000
Subject: [PATCH 07/17] Add task/reminder for future refactor.

---
 umpost/um2netcdf.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/umpost/um2netcdf.py b/umpost/um2netcdf.py
index d840926..774bf44 100644
--- a/umpost/um2netcdf.py
+++ b/umpost/um2netcdf.py
@@ -147,6 +147,7 @@ def _add_coord_bounds(coord):
 
 # TODO: split cube ops into functions, this will likely increase process() workflow steps
 def cubewrite(cube, sman, compression, use64bit, verbose):
+    # TODO: move into process() AND if a new cube is returned, swap into filtered cube list
     cube = fix_plevs(cube) or cube  # NB: use new cube if pressure points are modified
 
     if not use64bit:

From 09c66c039e3824074b23ab51be5b09884c17b1e6 Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Fri, 30 Aug 2024 12:37:22 +1000
Subject: [PATCH 08/17] Refactor: rename to pressure levels.

---
 test/test_um2netcdf.py | 6 +++---
 umpost/um2netcdf.py    | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py
index 3fee38b..3acb190 100644
--- a/test/test_um2netcdf.py
+++ b/test/test_um2netcdf.py
@@ -759,7 +759,7 @@ def test_fix_plevs_no_pressure_coord(get_fake_cube_coords):
     with pytest.raises(iris.exceptions.CoordinateNotFoundError):
         cube.coord("pressure")  # ensure missing 'pressure' coord
 
-    um2nc.fix_plevs(cube)  # should just exit
+    um2nc.fix_pressure_levels(cube)  # should just exit
 
 
 def _add_attrs_points(m_plevs: mock.MagicMock, points):
@@ -775,7 +775,7 @@ def test_fix_plevs_do_rounding(get_fake_cube_coords):
     extra = {"pressure": m_plevs}
     cube = get_fake_cube_coords(extra)
 
-    um2nc.fix_plevs(cube)
+    um2nc.fix_pressure_levels(cube)
 
     # TODO: test flaw, this verifies pressure coord but ignores fix_plevs()
     #       returning a new cube if the pressure is reversed. This is verified
@@ -792,7 +792,7 @@ def test_fix_plevs_reverse_pressure(get_fake_cube_coords):
     cube = get_fake_cube_coords(extra)
 
     with mock.patch("iris.util.reverse"):
-        um2nc.fix_plevs(cube)
+        um2nc.fix_pressure_levels(cube)
 
     # TODO: test flaw, this verifies pressure coord but ignores fix_plevs()
     #       returning a new cube if the pressure is reversed. This is verified
diff --git a/umpost/um2netcdf.py b/umpost/um2netcdf.py
index 774bf44..f59e750 100644
--- a/umpost/um2netcdf.py
+++ b/umpost/um2netcdf.py
@@ -148,7 +148,7 @@ def _add_coord_bounds(coord):
 # TODO: split cube ops into functions, this will likely increase process() workflow steps
 def cubewrite(cube, sman, compression, use64bit, verbose):
     # TODO: move into process() AND if a new cube is returned, swap into filtered cube list
-    cube = fix_plevs(cube) or cube  # NB: use new cube if pressure points are modified
+    cube = fix_pressure_levels(cube) or cube  # NB: use new cube if pressure points are modified
 
     if not use64bit:
         if cube.data.dtype == 'float64':
@@ -727,7 +727,7 @@ def fix_level_coord(cube, z_rho, z_theta, tol=1e-6):
                 c_sigma.var_name = 'sigma_theta'
 
 
-def fix_plevs(cube, decimals=5):
+def fix_pressure_levels(cube, decimals=5):
     """
     Reformat pressure level data for NetCDF output.
 

From e54c804cea310e71ebe6d6fdefca7d7e5898d8ae Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Fri, 30 Aug 2024 13:35:10 +1000
Subject: [PATCH 09/17] Refactor: rename variables.

---
 umpost/um2netcdf.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/umpost/um2netcdf.py b/umpost/um2netcdf.py
index f59e750..d17c540 100644
--- a/umpost/um2netcdf.py
+++ b/umpost/um2netcdf.py
@@ -745,18 +745,18 @@ def fix_pressure_levels(cube, decimals=5):
     cube if the pressure levels are reversed.
     """
     try:
-        plevs = cube.coord('pressure')
+        pressure = cube.coord('pressure')
     except iris.exceptions.CoordinateNotFoundError:
         return
 
     # update existing cube metadata in place
-    plevs.attributes['positive'] = 'down'
-    plevs.convert_units('Pa')
+    pressure.attributes['positive'] = 'down'
+    pressure.convert_units('Pa')
 
     # Round small fractions otherwise coordinates are off by 1e-10 in ncdump output
-    plevs.points = np.round(plevs.points, decimals)
+    pressure.points = np.round(pressure.points, decimals)
 
-    if plevs.points[0] < plevs.points[-1]:
+    if pressure.points[0] < pressure.points[-1]:
         # Flip to get pressure decreasing as per CMIP6 standard
         # NOTE: returns a new cube!
         return iris.util.reverse(cube, 'pressure')

From 9c53c06c7ad6784b1600d75323d4aabac9d89515 Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Fri, 30 Aug 2024 13:41:19 +1000
Subject: [PATCH 10/17] Refactor: rename tests & variables.

---
 test/test_um2netcdf.py | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py
index 3acb190..6b7bbce 100644
--- a/test/test_um2netcdf.py
+++ b/test/test_um2netcdf.py
@@ -753,7 +753,7 @@ 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_plevs_no_pressure_coord(get_fake_cube_coords):
+def test_fix_pressure_levels_no_pressure_coord(get_fake_cube_coords):
     cube = get_fake_cube_coords()
 
     with pytest.raises(iris.exceptions.CoordinateNotFoundError):
@@ -769,10 +769,10 @@ def _add_attrs_points(m_plevs: mock.MagicMock, points):
     setattr(m_plevs, "points", points)
 
 
-def test_fix_plevs_do_rounding(get_fake_cube_coords):
-    m_plevs = mock.Mock()
-    _add_attrs_points(m_plevs, [1.000001, 0.000001])
-    extra = {"pressure": m_plevs}
+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)
 
     um2nc.fix_pressure_levels(cube)
@@ -780,15 +780,15 @@ def test_fix_plevs_do_rounding(get_fake_cube_coords):
     # TODO: test flaw, this verifies pressure coord but ignores fix_plevs()
     #       returning a new cube if the pressure is reversed. This is verified
     #       in command line testing though
-    plev = cube.coord('pressure')
-    assert plev.attributes["positive"] == "down"
-    assert all(plev.points == [1.0, 0.0])
+    c_pressure = cube.coord('pressure')
+    assert c_pressure.attributes["positive"] == "down"
+    assert all(c_pressure.points == [1.0, 0.0])
 
 
-def test_fix_plevs_reverse_pressure(get_fake_cube_coords):
-    m_plevs = mock.Mock()
-    _add_attrs_points(m_plevs, [0.000001, 1.000001])
-    extra = {"pressure": m_plevs}
+def test_fix_pressure_levels_reverse_pressure(get_fake_cube_coords):
+    m_pressure = mock.Mock()
+    _add_attrs_points(m_pressure, [0.000001, 1.000001])
+    extra = {"pressure": m_pressure}
     cube = get_fake_cube_coords(extra)
 
     with mock.patch("iris.util.reverse"):
@@ -797,6 +797,6 @@ def test_fix_plevs_reverse_pressure(get_fake_cube_coords):
     # TODO: test flaw, this verifies pressure coord but ignores fix_plevs()
     #       returning a new cube if the pressure is reversed. This is verified
     #       in command line testing though
-    plev = cube.coord('pressure')
-    assert plev.attributes["positive"] == "down"
-    assert all(plev.points == [0.0, 1.0])
+    c_pressure = cube.coord('pressure')
+    assert c_pressure.attributes["positive"] == "down"
+    assert all(c_pressure.points == [0.0, 1.0])

From d8da6b1fe9ff8080f9c2f451bc28232a4316770e Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Fri, 30 Aug 2024 13:43:24 +1000
Subject: [PATCH 11/17] Update comments.

---
 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 6b7bbce..bb3442d 100644
--- a/test/test_um2netcdf.py
+++ b/test/test_um2netcdf.py
@@ -777,7 +777,7 @@ def test_fix_pressure_levels_do_rounding(get_fake_cube_coords):
 
     um2nc.fix_pressure_levels(cube)
 
-    # TODO: test flaw, this verifies pressure coord but ignores fix_plevs()
+    # TODO: test flaw, this verifies pressure coord but ignores fix_pressure_levels()
     #       returning a new cube if the pressure is reversed. This is verified
     #       in command line testing though
     c_pressure = cube.coord('pressure')
@@ -794,7 +794,7 @@ def test_fix_pressure_levels_reverse_pressure(get_fake_cube_coords):
     with mock.patch("iris.util.reverse"):
         um2nc.fix_pressure_levels(cube)
 
-    # TODO: test flaw, this verifies pressure coord but ignores fix_plevs()
+    # TODO: test flaw, this verifies pressure coord but ignores fix_pressure_levels()
     #       returning a new cube if the pressure is reversed. This is verified
     #       in command line testing though
     c_pressure = cube.coord('pressure')

From dd9c18dd1ad709bd7c3dd839d3eb74bc6ebbfae4 Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Fri, 20 Sep 2024 11:10:00 +1000
Subject: [PATCH 12/17] Add explicit None assertion for pressure test.

---
 test/test_um2netcdf.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py
index 312a862..b6f4727 100644
--- a/test/test_um2netcdf.py
+++ b/test/test_um2netcdf.py
@@ -761,7 +761,8 @@ def test_fix_pressure_levels_no_pressure_coord(get_fake_cube_coords):
     with pytest.raises(iris.exceptions.CoordinateNotFoundError):
         cube.coord("pressure")  # ensure missing 'pressure' coord
 
-    um2nc.fix_pressure_levels(cube)  # should just exit
+    # fix function should return if there is no pressure coord to modify
+    assert um2nc.fix_pressure_levels(cube) is None  # should just exit
 
 
 def _add_attrs_points(m_plevs: mock.MagicMock, points):

From b1042bee46b9e7de6a9f942c7930a9dcc96cf61f Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Tue, 24 Sep 2024 14:13:20 +1000
Subject: [PATCH 13/17] Add explicit None assertion for fix pressure rounding
 test.

---
 test/test_um2netcdf.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py
index b6f4727..e26e9e6 100644
--- a/test/test_um2netcdf.py
+++ b/test/test_um2netcdf.py
@@ -778,7 +778,8 @@ def test_fix_pressure_levels_do_rounding(get_fake_cube_coords):
     extra = {"pressure": m_pressure}
     cube = get_fake_cube_coords(extra)
 
-    um2nc.fix_pressure_levels(cube)
+    # ensure no cube is returned if Cube not modified in fix_pressure_levels()
+    assert um2nc.fix_pressure_levels(cube) is None
 
     # TODO: test flaw, this verifies pressure coord but ignores fix_pressure_levels()
     #       returning a new cube if the pressure is reversed. This is verified

From 1941c6933829402b81682cd06eef4f5df32bead8 Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Tue, 24 Sep 2024 15:02:04 +1000
Subject: [PATCH 14/17] Skip pressure reversal test & add reasoning comments.

---
 test/test_um2netcdf.py | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py
index e26e9e6..56f84e1 100644
--- a/test/test_um2netcdf.py
+++ b/test/test_um2netcdf.py
@@ -781,29 +781,46 @@ def test_fix_pressure_levels_do_rounding(get_fake_cube_coords):
     # ensure no cube is returned if Cube not modified in fix_pressure_levels()
     assert um2nc.fix_pressure_levels(cube) is None
 
-    # TODO: test flaw, this verifies pressure coord but ignores fix_pressure_levels()
-    #       returning a new cube if the pressure is reversed. This is verified
-    #       in command line testing though
     c_pressure = cube.coord('pressure')
     assert c_pressure.attributes["positive"] == "down"
     assert all(c_pressure.points == [1.0, 0.0])
 
 
+@pytest.mark.skip
 def test_fix_pressure_levels_reverse_pressure(get_fake_cube_coords):
+    # TODO: test is broken, it verifies the pressure coord but doesn't handle
+    #       the real fix_pressure_levels() returning a new cube when the pressure
+    #       is reversed.
+
     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
+
+    # TODO: testing gets odd here at the um2nc & iris "boundary":
+    #   * A mock reverse() needs to flip pressure.points & return a modified cube.
+    #     Creating a mock to verifying these attributes is unproductive.
+    #   * Using the real reverse() requires several additional cube attributes
+    #     (see commented out ndim etc above). It requires __getitem__() for
+    #     https://github.com/SciTools/iris/blob/main/lib/iris/util.py#L612
+    #
+    # TODO: this leaves a few options:
+    #    * ignore unit testing this branch (rely on integration testing?)
+    #    * replace iris with an adapter?
+    #    * fix/refactor the function later?
+    #
+    # The test is disabled awaiting a solution...
 
     with mock.patch("iris.util.reverse"):
-        um2nc.fix_pressure_levels(cube)
+        mod_cube = um2nc.fix_pressure_levels(cube)
 
-    # TODO: test flaw, this verifies pressure coord but ignores fix_pressure_levels()
-    #       returning a new cube if the pressure is reversed. This is verified
-    #       in command line testing though
-    c_pressure = cube.coord('pressure')
+    assert mod_cube is not None
+    assert mod_cube != cube
+    c_pressure = mod_cube.coord('pressure')
     assert c_pressure.attributes["positive"] == "down"
-    assert all(c_pressure.points == [0.0, 1.0])
+    assert all(c_pressure.points == [1.0, 0.0])
 
 
 # int64 to int32 data conversion tests

From 48855673dadf25e24ed2171c802bd20e96117b41 Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Tue, 24 Sep 2024 15:03:30 +1000
Subject: [PATCH 15/17] Skip coverage of external versioneer module.

---
 .coveragerc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.coveragerc b/.coveragerc
index bf9b06d..310af46 100644
--- a/.coveragerc
+++ b/.coveragerc
@@ -1,6 +1,9 @@
 [run]
 branch = True
 
+# skip coverage testing for external source files
+omit = umpost/_version.py
+
 [html]
 title = Coverage report: um2nc-standalone
 directory = coverage_html

From 4fd75dfd8be2b92058cc38765214495aef13c4da Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Tue, 24 Sep 2024 15:05:39 +1000
Subject: [PATCH 16/17] Fix comment.

---
 test/test_um2netcdf.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/test/test_um2netcdf.py b/test/test_um2netcdf.py
index 56f84e1..a140061 100644
--- a/test/test_um2netcdf.py
+++ b/test/test_um2netcdf.py
@@ -788,9 +788,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):
-    # TODO: test is broken, it verifies the pressure coord but doesn't handle
-    #       the real fix_pressure_levels() returning a new cube when the pressure
-    #       is reversed.
+    # TODO: test is broken due to fiddly mocking problems (see below)
 
     m_pressure = mock.Mock()
     # m_pressure.ndim = 1

From f6ba005c41f64d5f9a56496b297136d890582e31 Mon Sep 17 00:00:00 2001
From: Ben Davies <ben.davies@anu.edu.au>
Date: Tue, 24 Sep 2024 15:10:35 +1000
Subject: [PATCH 17/17] Add TODO for potential monotonic checking feature.

---
 umpost/um2netcdf.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/umpost/um2netcdf.py b/umpost/um2netcdf.py
index ba4b9cc..618d66c 100644
--- a/umpost/um2netcdf.py
+++ b/umpost/um2netcdf.py
@@ -757,6 +757,7 @@ def fix_pressure_levels(cube, decimals=5):
     if pressure.points[0] < pressure.points[-1]:
         # Flip to get pressure decreasing as per CMIP6 standard
         # NOTE: returns a new cube!
+        # TODO: add an iris.util.monotonic() check here?
         return iris.util.reverse(cube, 'pressure')