From 21477fb35b9b26583923a2f786379d4d321da7cc Mon Sep 17 00:00:00 2001
From: Manuel Schlund <manuel.schlund@dlr.de>
Date: Fri, 18 Oct 2024 08:50:20 +0200
Subject: [PATCH 1/6] Add safe unit conversion (no change of standard_name
 allowed)

---
 esmvalcore/preprocessor/_units.py             | 84 +++++++++++++++----
 .../preprocessor/_units/test_convert_units.py | 65 +++++++++++++-
 2 files changed, 131 insertions(+), 18 deletions(-)

diff --git a/esmvalcore/preprocessor/_units.py b/esmvalcore/preprocessor/_units.py
index 23ebe23cc2..8531d6f0d2 100644
--- a/esmvalcore/preprocessor/_units.py
+++ b/esmvalcore/preprocessor/_units.py
@@ -11,6 +11,8 @@
 import iris
 import numpy as np
 from cf_units import Unit
+from iris.coords import AuxCoord, DimCoord
+from iris.cube import Cube
 
 logger = logging.getLogger(__name__)
 
@@ -32,7 +34,7 @@
 ]
 
 
-def _try_special_conversions(cube, units):
+def _try_special_conversions(cube: Cube, units: str | Unit) -> bool:
     """Try special conversion."""
     for special_case in SPECIAL_CASES:
         for std_name, special_units in special_case:
@@ -72,16 +74,15 @@ def _try_special_conversions(cube, units):
     return False
 
 
-def convert_units(cube, units):
-    """Convert the units of a cube to new ones.
-
-    This converts units of a cube.
+def convert_units(cube: Cube, units: str | Unit) -> Cube:
+    """Convert the units of a cube to new ones (in-place).
 
     Note
     ----
     Allows special unit conversions which transforms one quantity to another
-    (physically related) quantity. These quantities are identified via their
-    ``standard_name`` and their ``units`` (units convertible to the ones
+    (physically related) quantity, which may also change the input cube's
+    :attr:`~iris.cube.Cube.standard_name`. These quantities are identified via
+    their ``standard_name`` and their ``units`` (units convertible to the ones
     defined are also supported). For example, this enables conversions between
     precipitation fluxes measured in ``kg m-2 s-1`` and precipitation rates
     measured in ``mm day-1`` (and vice versa).
@@ -105,15 +106,23 @@ def convert_units(cube, units):
 
     Arguments
     ---------
-    cube: iris.cube.Cube
+    cube:
         Input cube.
-    units: str
-        New units in udunits form.
+    units:
+        New units.
 
     Returns
     -------
     iris.cube.Cube
-        converted cube.
+        Converted cube. Just returned for convenience; input cube is modified
+        in place.
+
+    Raises
+    ------
+    iris.exceptions.UnitConversionError
+        Old units are unknown.
+    ValueError
+        Old units are not convertible to new units.
 
     """
     try:
@@ -125,10 +134,51 @@ def convert_units(cube, units):
     return cube
 
 
+def safe_convert_units(cube: Cube, units: str | Unit) -> Cube:
+    """Safe unit conversion (change of `standard_name` not allowed; in-place).
+
+    This is a safe version of :func:`esmvalcore.preprocessor.convert_units`
+    that will raise an error if the input cube's
+    :attr:`~iris.cube.Cube.standard_name` has been changed.
+
+    Arguments
+    ---------
+    cube:
+        Input cube.
+    units:
+        New units.
+
+    Returns
+    -------
+    iris.cube.Cube
+        Converted cube. Just returned for convenience; input cube is modified
+        in place.
+
+    Raises
+    ------
+    iris.exceptions.UnitConversionError
+        Old units are unknown.
+    ValueError
+        Old units are not convertible to new units or unit conversion required
+        change of `standard_name`.
+
+    """
+    old_units = cube.units
+    old_standard_name = cube.standard_name
+    cube = convert_units(cube, units)
+    if cube.standard_name != old_standard_name:
+        raise ValueError(
+            f"Cannot safely convert units from '{old_units}' to '{units}'; "
+            f"standard_name changed from '{old_standard_name}' to "
+            f"'{cube.standard_name}'"
+        )
+    return cube
+
+
 def accumulate_coordinate(
-    cube: iris.cube.Cube,
-    coordinate: str | iris.coords.DimCoord | iris.coords.AuxCoord,
-) -> iris.cube.Cube:
+    cube: Cube,
+    coordinate: str | DimCoord | AuxCoord,
+) -> Cube:
     """Weight data using the bounds from a given coordinate.
 
     The resulting cube will then have units given by
@@ -137,7 +187,7 @@ def accumulate_coordinate(
     Parameters
     ----------
     cube:
-        Data cube for the flux
+        Data cube for the flux.
 
     coordinate:
         Name of the coordinate that will be used as weights.
@@ -145,7 +195,7 @@ def accumulate_coordinate(
     Returns
     -------
     iris.cube.Cube
-        Cube with the aggregated data
+        Cube with the aggregated data.
 
     Raises
     ------
@@ -169,7 +219,7 @@ def accumulate_coordinate(
         )
 
     array_module = da if coord.has_lazy_bounds() else np
-    factor = iris.coords.AuxCoord(
+    factor = AuxCoord(
         array_module.diff(coord.core_bounds())[..., -1],
         var_name=coord.var_name,
         long_name=coord.long_name,
diff --git a/tests/unit/preprocessor/_units/test_convert_units.py b/tests/unit/preprocessor/_units/test_convert_units.py
index 8fea071943..3c60447db3 100644
--- a/tests/unit/preprocessor/_units/test_convert_units.py
+++ b/tests/unit/preprocessor/_units/test_convert_units.py
@@ -6,9 +6,16 @@
 import iris
 import iris.fileformats
 import numpy as np
+import pytest
+from iris.cube import Cube
+from iris.exceptions import UnitConversionError
 
 import tests
-from esmvalcore.preprocessor._units import accumulate_coordinate, convert_units
+from esmvalcore.preprocessor._units import (
+    accumulate_coordinate,
+    convert_units,
+    safe_convert_units,
+)
 
 
 class TestConvertUnits(tests.Test):
@@ -257,5 +264,61 @@ def test_flux_by_hour(self):
         self.assert_array_equal(result.data, expected_data)
 
 
+@pytest.mark.parametrize(
+    "old_units,new_units,old_standard_name,new_standard_name,err_msg",
+    [
+        ("m", "km", "altitude", "altitude", None),
+        ("Pa", "hPa", "air_pressure", "air_pressure", None),
+        (
+            "m",
+            "DU",
+            "equivalent_thickness_at_stp_of_atmosphere_ozone_content",
+            "equivalent_thickness_at_stp_of_atmosphere_ozone_content",
+            None,
+        ),
+        (
+            "m",
+            "s",
+            "altitude",
+            ValueError,
+            r"Unable to convert from 'Unit\('m'\)' to 'Unit\('s'\)'",
+        ),
+        (
+            "unknown",
+            "s",
+            "air_temperature",
+            UnitConversionError,
+            r"Cannot convert from unknown units",
+        ),
+        (
+            "kg m-2 s-1",
+            "mm day-1",
+            "precipitation_flux",
+            ValueError,
+            r"Cannot safely convert units from 'kg m-2 s-1' to 'mm day-1'; "
+            r"standard_name changed from 'precipitation_flux' to "
+            r"'lwe_precipitation_rate'",
+        ),
+    ],
+)
+def test_safe_convert_units(
+    old_units, new_units, old_standard_name, new_standard_name, err_msg
+):
+    """Test ``esmvalcore.preprocessor._units.safe_convert_units``."""
+    cube = Cube(0, standard_name=old_standard_name, units=old_units)
+
+    # Exceptions
+    if isinstance(new_standard_name, type):
+        with pytest.raises(new_standard_name, match=err_msg):
+            safe_convert_units(cube, new_units)
+        return
+
+    # Regular test cases
+    new_cube = safe_convert_units(cube, new_units)
+    assert new_cube is cube
+    assert new_cube.standard_name == new_standard_name
+    assert new_cube.units == new_units
+
+
 if __name__ == "__main__":
     unittest.main()

From 31a0dbd1ebeaeb566dc1458f660138d58af301f2 Mon Sep 17 00:00:00 2001
From: Manuel Schlund <manuel.schlund@dlr.de>
Date: Fri, 18 Oct 2024 08:52:36 +0200
Subject: [PATCH 2/6] Use safe unit conversion in fixes

---
 esmvalcore/cmor/_fixes/fix.py             | 3 ++-
 esmvalcore/cmor/_fixes/native6/era5.py    | 7 ++-----
 esmvalcore/cmor/_fixes/native_datasets.py | 4 +++-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/esmvalcore/cmor/_fixes/fix.py b/esmvalcore/cmor/_fixes/fix.py
index 973ac57d0b..0d3f705ae1 100644
--- a/esmvalcore/cmor/_fixes/fix.py
+++ b/esmvalcore/cmor/_fixes/fix.py
@@ -28,6 +28,7 @@
 from esmvalcore.cmor.fixes import get_time_bounds
 from esmvalcore.cmor.table import get_var_info
 from esmvalcore.iris_helpers import has_unstructured_grid
+from esmvalcore.preprocessor._units import safe_convert_units
 
 if TYPE_CHECKING:
     from esmvalcore.cmor.table import CoordinateInfo, VariableInfo
@@ -455,7 +456,7 @@ def _fix_units(self, cube: Cube) -> Cube:
             if str(cube.units) != units:
                 old_units = cube.units
                 try:
-                    cube.convert_units(units)
+                    safe_convert_units(cube, units)
                 except (ValueError, UnitConversionError):
                     self._warning_msg(
                         cube,
diff --git a/esmvalcore/cmor/_fixes/native6/era5.py b/esmvalcore/cmor/_fixes/native6/era5.py
index aa4beae984..719cffd1d7 100644
--- a/esmvalcore/cmor/_fixes/native6/era5.py
+++ b/esmvalcore/cmor/_fixes/native6/era5.py
@@ -7,6 +7,7 @@
 import numpy as np
 
 from esmvalcore.iris_helpers import date2num
+from esmvalcore.preprocessor._units import safe_convert_units
 
 from ...table import CMOR_TABLES
 from ..fix import Fix
@@ -414,10 +415,6 @@ def _fix_monthly_time_coord(cube):
             coord.points = 0.5 * (start + end)
             coord.bounds = np.column_stack([start, end])
 
-    def _fix_units(self, cube):
-        """Fix units."""
-        cube.convert_units(self.vardef.units)
-
     def fix_metadata(self, cubes):
         """Fix metadata."""
         fixed_cubes = iris.cube.CubeList()
@@ -428,7 +425,7 @@ def fix_metadata(self, cubes):
             cube.long_name = self.vardef.long_name
 
             cube = self._fix_coordinates(cube)
-            self._fix_units(cube)
+            cube = safe_convert_units(cube, self.vardef.units)
 
             cube.data = cube.core_data().astype("float32")
             year = datetime.datetime.now().year
diff --git a/esmvalcore/cmor/_fixes/native_datasets.py b/esmvalcore/cmor/_fixes/native_datasets.py
index 6b48d92f54..219e92768b 100644
--- a/esmvalcore/cmor/_fixes/native_datasets.py
+++ b/esmvalcore/cmor/_fixes/native_datasets.py
@@ -5,6 +5,8 @@
 
 from iris import NameConstraint
 
+from esmvalcore.preprocessor._units import safe_convert_units
+
 from ..fix import Fix
 from .shared import (
     add_scalar_height_coord,
@@ -77,7 +79,7 @@ def fix_var_metadata(self, cube):
                     f"Failed to fix invalid units '{invalid_units}' for "
                     f"variable '{self.vardef.short_name}'"
                 ) from exc
-        cube.convert_units(self.vardef.units)
+        safe_convert_units(cube, self.vardef.units)
 
         # Fix attributes
         if self.vardef.positive != "":

From 4976b3c7154b0f14e6f187b5e2f5996894058190 Mon Sep 17 00:00:00 2001
From: Manuel Schlund <manuel.schlund@dlr.de>
Date: Fri, 18 Oct 2024 09:38:12 +0200
Subject: [PATCH 3/6] Avoid circular import

---
 esmvalcore/cmor/_fixes/fix.py                 |   3 +-
 esmvalcore/cmor/_fixes/native6/era5.py        |   3 +-
 esmvalcore/cmor/_fixes/native_datasets.py     |   2 +-
 esmvalcore/iris_helpers.py                    | 119 ++++++++++++++++++
 esmvalcore/preprocessor/_units.py             | 106 +---------------
 .../preprocessor/_units/test_convert_units.py |  66 ----------
 tests/unit/test_iris_helpers.py               |  59 ++++++++-
 7 files changed, 185 insertions(+), 173 deletions(-)

diff --git a/esmvalcore/cmor/_fixes/fix.py b/esmvalcore/cmor/_fixes/fix.py
index 0d3f705ae1..4d3e297e3a 100644
--- a/esmvalcore/cmor/_fixes/fix.py
+++ b/esmvalcore/cmor/_fixes/fix.py
@@ -27,8 +27,7 @@
 )
 from esmvalcore.cmor.fixes import get_time_bounds
 from esmvalcore.cmor.table import get_var_info
-from esmvalcore.iris_helpers import has_unstructured_grid
-from esmvalcore.preprocessor._units import safe_convert_units
+from esmvalcore.iris_helpers import has_unstructured_grid, safe_convert_units
 
 if TYPE_CHECKING:
     from esmvalcore.cmor.table import CoordinateInfo, VariableInfo
diff --git a/esmvalcore/cmor/_fixes/native6/era5.py b/esmvalcore/cmor/_fixes/native6/era5.py
index 719cffd1d7..79fa2856fd 100644
--- a/esmvalcore/cmor/_fixes/native6/era5.py
+++ b/esmvalcore/cmor/_fixes/native6/era5.py
@@ -6,8 +6,7 @@
 import iris
 import numpy as np
 
-from esmvalcore.iris_helpers import date2num
-from esmvalcore.preprocessor._units import safe_convert_units
+from esmvalcore.iris_helpers import date2num, safe_convert_units
 
 from ...table import CMOR_TABLES
 from ..fix import Fix
diff --git a/esmvalcore/cmor/_fixes/native_datasets.py b/esmvalcore/cmor/_fixes/native_datasets.py
index 219e92768b..da6f470816 100644
--- a/esmvalcore/cmor/_fixes/native_datasets.py
+++ b/esmvalcore/cmor/_fixes/native_datasets.py
@@ -5,7 +5,7 @@
 
 from iris import NameConstraint
 
-from esmvalcore.preprocessor._units import safe_convert_units
+from esmvalcore.iris_helpers import safe_convert_units
 
 from ..fix import Fix
 from .shared import (
diff --git a/esmvalcore/iris_helpers.py b/esmvalcore/iris_helpers.py
index 4162233eec..7c6ee2145e 100644
--- a/esmvalcore/iris_helpers.py
+++ b/esmvalcore/iris_helpers.py
@@ -9,6 +9,7 @@
 import iris.cube
 import iris.util
 import numpy as np
+from cf_units import Unit
 from iris.coords import Coord
 from iris.cube import Cube
 from iris.exceptions import CoordinateMultiDimError, CoordinateNotFoundError
@@ -358,3 +359,121 @@ def has_unstructured_grid(cube: Cube) -> bool:
     if cube.coord_dims(lat) != cube.coord_dims(lon):
         return False
     return True
+
+
+# List containing special cases for convert_units. Each list item is another
+# list. Each of these sublists defines one special conversion. Each element in
+# the sublists is a tuple (standard_name, units). Note: All units for a single
+# special case need to be "physically identical", e.g., 1 kg m-2 s-1 "equals" 1
+# mm s-1 for precipitation
+_SPECIAL_UNIT_CONVERSIONS = [
+    [
+        ("precipitation_flux", "kg m-2 s-1"),
+        ("lwe_precipitation_rate", "mm s-1"),
+    ],
+    [
+        ("equivalent_thickness_at_stp_of_atmosphere_ozone_content", "m"),
+        ("equivalent_thickness_at_stp_of_atmosphere_ozone_content", "1e5 DU"),
+    ],
+]
+
+
+def _try_special_unit_conversions(cube: Cube, units: str | Unit) -> bool:
+    """Try special unit conversion (in-place).
+
+    Parameters
+    ----------
+    cube:
+        Input cube (modified in place).
+    units:
+        New units
+
+    Returns
+    -------
+    bool
+        ``True`` if special unit conversion was successful, ``False`` if not.
+
+    """
+    for special_case in _SPECIAL_UNIT_CONVERSIONS:
+        for std_name, special_units in special_case:
+            # Special unit conversion only works if all of the following
+            # criteria are met:
+            # - the cube's standard_name is one of the supported
+            #   standard_names
+            # - the cube's units are convertible to the ones defined for
+            #   that given standard_name
+            # - the desired target units are convertible to the units of
+            #   one of the other standard_names in that special case
+
+            # Step 1: find suitable source name and units
+            if cube.standard_name == std_name and cube.units.is_convertible(
+                special_units
+            ):
+                for target_std_name, target_units in special_case:
+                    if target_units == special_units:
+                        continue
+
+                    # Step 2: find suitable target name and units
+                    if Unit(units).is_convertible(target_units):
+                        cube.standard_name = target_std_name
+
+                        # In order to avoid two calls to cube.convert_units,
+                        # determine the conversion factor between the cube's
+                        # units and the source units first and simply add this
+                        # factor to the target units (remember that the source
+                        # units and the target units should be "physically
+                        # identical").
+                        factor = cube.units.convert(1.0, special_units)
+                        cube.units = f"{factor} {target_units}"
+                        cube.convert_units(units)
+                        return True
+
+    # If no special case has been detected, return False
+    return False
+
+
+def safe_convert_units(cube: Cube, units: str | Unit) -> Cube:
+    """Safe unit conversion (change of `standard_name` not allowed; in-place).
+
+    This is a safe version of :func:`esmvalcore.preprocessor.convert_units`
+    that will raise an error if the input cube's
+    :attr:`~iris.cube.Cube.standard_name` has been changed.
+
+    Arguments
+    ---------
+    cube:
+        Input cube (modified in place).
+    units:
+        New units.
+
+    Returns
+    -------
+    iris.cube.Cube
+        Converted cube. Just returned for convenience; input cube is modified
+        in place.
+
+    Raises
+    ------
+    iris.exceptions.UnitConversionError
+        Old units are unknown.
+    ValueError
+        Old units are not convertible to new units or unit conversion required
+        change of `standard_name`.
+
+    """
+    old_units = cube.units
+    old_standard_name = cube.standard_name
+
+    try:
+        cube.convert_units(units)
+    except ValueError:
+        if not _try_special_unit_conversions(cube, units):
+            raise
+
+    if cube.standard_name != old_standard_name:
+        raise ValueError(
+            f"Cannot safely convert units from '{old_units}' to '{units}'; "
+            f"standard_name changed from '{old_standard_name}' to "
+            f"'{cube.standard_name}'"
+        )
+    return cube
diff --git a/esmvalcore/preprocessor/_units.py b/esmvalcore/preprocessor/_units.py
index 8531d6f0d2..8e0c764b78 100644
--- a/esmvalcore/preprocessor/_units.py
+++ b/esmvalcore/preprocessor/_units.py
@@ -14,64 +14,9 @@
 from iris.coords import AuxCoord, DimCoord
 from iris.cube import Cube
 
-logger = logging.getLogger(__name__)
-
+from esmvalcore.iris_helpers import _try_special_unit_conversions
 
-# List containing special cases for convert_units. Each list item is another
-# list. Each of these sublists defines one special conversion. Each element in
-# the sublists is a tuple (standard_name, units). Note: All units for a single
-# special case need to be "physically identical", e.g., 1 kg m-2 s-1 "equals" 1
-# mm s-1 for precipitation
-SPECIAL_CASES = [
-    [
-        ("precipitation_flux", "kg m-2 s-1"),
-        ("lwe_precipitation_rate", "mm s-1"),
-    ],
-    [
-        ("equivalent_thickness_at_stp_of_atmosphere_ozone_content", "m"),
-        ("equivalent_thickness_at_stp_of_atmosphere_ozone_content", "1e5 DU"),
-    ],
-]
-
-
-def _try_special_conversions(cube: Cube, units: str | Unit) -> bool:
-    """Try special conversion."""
-    for special_case in SPECIAL_CASES:
-        for std_name, special_units in special_case:
-            # Special unit conversion only works if all of the following
-            # criteria are met:
-            # - the cube's standard_name is one of the supported
-            #   standard_names
-            # - the cube's units are convertible to the ones defined for
-            #   that given standard_name
-            # - the desired target units are convertible to the units of
-            #   one of the other standard_names in that special case
-
-            # Step 1: find suitable source name and units
-            if cube.standard_name == std_name and cube.units.is_convertible(
-                special_units
-            ):
-                for target_std_name, target_units in special_case:
-                    if target_units == special_units:
-                        continue
-
-                    # Step 2: find suitable target name and units
-                    if Unit(units).is_convertible(target_units):
-                        cube.standard_name = target_std_name
-
-                        # In order to avoid two calls to cube.convert_units,
-                        # determine the conversion factor between the cube's
-                        # units and the source units first and simply add this
-                        # factor to the target units (remember that the source
-                        # units and the target units should be "physically
-                        # identical").
-                        factor = cube.units.convert(1.0, special_units)
-                        cube.units = f"{factor} {target_units}"
-                        cube.convert_units(units)
-                        return True
-
-    # If no special case has been detected, return False
-    return False
+logger = logging.getLogger(__name__)
 
 
 def convert_units(cube: Cube, units: str | Unit) -> Cube:
@@ -104,10 +49,10 @@ def convert_units(cube: Cube, units: str | Unit) -> Cube:
     Note that for precipitation variables, a water density of ``1000 kg m-3``
     is assumed.
 
-    Arguments
+    Argumentss
     ---------
     cube:
-        Input cube.
+        Input cube (modified in place).
     units:
         New units.
 
@@ -128,53 +73,12 @@ def convert_units(cube: Cube, units: str | Unit) -> Cube:
     try:
         cube.convert_units(units)
     except ValueError:
-        if not _try_special_conversions(cube, units):
+        if not _try_special_unit_conversions(cube, units):
             raise
 
     return cube
 
 
-def safe_convert_units(cube: Cube, units: str | Unit) -> Cube:
-    """Safe unit conversion (change of `standard_name` not allowed; in-place).
-
-    This is a safe version of :func:`esmvalcore.preprocessor.convert_units`
-    that will raise an error if the input cube's
-    :attr:`~iris.cube.Cube.standard_name` has been changed.
-
-    Arguments
-    ---------
-    cube:
-        Input cube.
-    units:
-        New units.
-
-    Returns
-    -------
-    iris.cube.Cube
-        Converted cube. Just returned for convenience; input cube is modified
-        in place.
-
-    Raises
-    ------
-    iris.exceptions.UnitConversionError
-        Old units are unknown.
-    ValueError
-        Old units are not convertible to new units or unit conversion required
-        change of `standard_name`.
-
-    """
-    old_units = cube.units
-    old_standard_name = cube.standard_name
-    cube = convert_units(cube, units)
-    if cube.standard_name != old_standard_name:
-        raise ValueError(
-            f"Cannot safely convert units from '{old_units}' to '{units}'; "
-            f"standard_name changed from '{old_standard_name}' to "
-            f"'{cube.standard_name}'"
-        )
-    return cube
-
-
 def accumulate_coordinate(
     cube: Cube,
     coordinate: str | DimCoord | AuxCoord,
diff --git a/tests/unit/preprocessor/_units/test_convert_units.py b/tests/unit/preprocessor/_units/test_convert_units.py
index 3c60447db3..a56f6f5da0 100644
--- a/tests/unit/preprocessor/_units/test_convert_units.py
+++ b/tests/unit/preprocessor/_units/test_convert_units.py
@@ -1,20 +1,14 @@
 """Unit test for the :func:`esmvalcore.preprocessor._units` function."""
 
-import unittest
-
 import cf_units
 import iris
 import iris.fileformats
 import numpy as np
-import pytest
-from iris.cube import Cube
-from iris.exceptions import UnitConversionError
 
 import tests
 from esmvalcore.preprocessor._units import (
     accumulate_coordinate,
     convert_units,
-    safe_convert_units,
 )
 
 
@@ -262,63 +256,3 @@ def test_flux_by_hour(self):
         expected_units = cf_units.Unit("kg")
         self.assertEqual(result.units, expected_units)
         self.assert_array_equal(result.data, expected_data)
-
-
-@pytest.mark.parametrize(
-    "old_units,new_units,old_standard_name,new_standard_name,err_msg",
-    [
-        ("m", "km", "altitude", "altitude", None),
-        ("Pa", "hPa", "air_pressure", "air_pressure", None),
-        (
-            "m",
-            "DU",
-            "equivalent_thickness_at_stp_of_atmosphere_ozone_content",
-            "equivalent_thickness_at_stp_of_atmosphere_ozone_content",
-            None,
-        ),
-        (
-            "m",
-            "s",
-            "altitude",
-            ValueError,
-            r"Unable to convert from 'Unit\('m'\)' to 'Unit\('s'\)'",
-        ),
-        (
-            "unknown",
-            "s",
-            "air_temperature",
-            UnitConversionError,
-            r"Cannot convert from unknown units",
-        ),
-        (
-            "kg m-2 s-1",
-            "mm day-1",
-            "precipitation_flux",
-            ValueError,
-            r"Cannot safely convert units from 'kg m-2 s-1' to 'mm day-1'; "
-            r"standard_name changed from 'precipitation_flux' to "
-            r"'lwe_precipitation_rate'",
-        ),
-    ],
-)
-def test_safe_convert_units(
-    old_units, new_units, old_standard_name, new_standard_name, err_msg
-):
-    """Test ``esmvalcore.preprocessor._units.safe_convert_units``."""
-    cube = Cube(0, standard_name=old_standard_name, units=old_units)
-
-    # Exceptions
-    if isinstance(new_standard_name, type):
-        with pytest.raises(new_standard_name, match=err_msg):
-            safe_convert_units(cube, new_units)
-        return
-
-    # Regular test cases
-    new_cube = safe_convert_units(cube, new_units)
-    assert new_cube is cube
-    assert new_cube.standard_name == new_standard_name
-    assert new_cube.units == new_units
-
-
-if __name__ == "__main__":
-    unittest.main()
diff --git a/tests/unit/test_iris_helpers.py b/tests/unit/test_iris_helpers.py
index 1b5066ae51..fe56d2d068 100644
--- a/tests/unit/test_iris_helpers.py
+++ b/tests/unit/test_iris_helpers.py
@@ -18,7 +18,7 @@
     DimCoord,
 )
 from iris.cube import Cube, CubeList
-from iris.exceptions import CoordinateMultiDimError
+from iris.exceptions import CoordinateMultiDimError, UnitConversionError
 
 from esmvalcore.iris_helpers import (
     add_leading_dim_to_cube,
@@ -28,6 +28,7 @@
     has_unstructured_grid,
     merge_cube_attributes,
     rechunk_cube,
+    safe_convert_units,
 )
 
 
@@ -571,3 +572,59 @@ def test_has_unstructured_grid_true(lat_coord_1d, lon_coord_1d):
         aux_coords_and_dims=[(lat_coord_1d, 0), (lon_coord_1d, 0)],
     )
     assert has_unstructured_grid(cube) is True
+
+
+@pytest.mark.parametrize(
+    "old_units,new_units,old_standard_name,new_standard_name,err_msg",
+    [
+        ("m", "km", "altitude", "altitude", None),
+        ("Pa", "hPa", "air_pressure", "air_pressure", None),
+        (
+            "m",
+            "DU",
+            "equivalent_thickness_at_stp_of_atmosphere_ozone_content",
+            "equivalent_thickness_at_stp_of_atmosphere_ozone_content",
+            None,
+        ),
+        (
+            "m",
+            "s",
+            "altitude",
+            ValueError,
+            r"Unable to convert from 'Unit\('m'\)' to 'Unit\('s'\)'",
+        ),
+        (
+            "unknown",
+            "s",
+            "air_temperature",
+            UnitConversionError,
+            r"Cannot convert from unknown units",
+        ),
+        (
+            "kg m-2 s-1",
+            "mm day-1",
+            "precipitation_flux",
+            ValueError,
+            r"Cannot safely convert units from 'kg m-2 s-1' to 'mm day-1'; "
+            r"standard_name changed from 'precipitation_flux' to "
+            r"'lwe_precipitation_rate'",
+        ),
+    ],
+)
+def test_safe_convert_units(
+    old_units, new_units, old_standard_name, new_standard_name, err_msg
+):
+    """Test ``esmvalcore.preprocessor._units.safe_convert_units``."""
+    cube = Cube(0, standard_name=old_standard_name, units=old_units)
+
+    # Exceptions
+    if isinstance(new_standard_name, type):
+        with pytest.raises(new_standard_name, match=err_msg):
+            safe_convert_units(cube, new_units)
+        return
+
+    # Regular test cases
+    new_cube = safe_convert_units(cube, new_units)
+    assert new_cube is cube
+    assert new_cube.standard_name == new_standard_name
+    assert new_cube.units == new_units

From 35bf9f4915ae504b8424bfb50c4996151b35778b Mon Sep 17 00:00:00 2001
From: Manuel Schlund <manuel.schlund@dlr.de>
Date: Fri, 18 Oct 2024 09:40:42 +0200
Subject: [PATCH 4/6] Better docstring

---
 esmvalcore/iris_helpers.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/esmvalcore/iris_helpers.py b/esmvalcore/iris_helpers.py
index 7c6ee2145e..ff3fdf2ba7 100644
--- a/esmvalcore/iris_helpers.py
+++ b/esmvalcore/iris_helpers.py
@@ -361,7 +361,7 @@ def has_unstructured_grid(cube: Cube) -> bool:
     return True
 
 
-# List containing special cases for convert_units. Each list item is another
+# List containing special cases for unit conversion. Each list item is another
 # list. Each of these sublists defines one special conversion. Each element in
 # the sublists is a tuple (standard_name, units). Note: All units for a single
 # special case need to be "physically identical", e.g., 1 kg m-2 s-1 "equals" 1

From 266b4e165f16b427b949356d2d409afd5f49bfa2 Mon Sep 17 00:00:00 2001
From: Manuel Schlund <manuel.schlund@dlr.de>
Date: Fri, 18 Oct 2024 10:06:17 +0200
Subject: [PATCH 5/6] Fix doc build

---
 esmvalcore/iris_helpers.py        | 8 ++++----
 esmvalcore/preprocessor/_units.py | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/esmvalcore/iris_helpers.py b/esmvalcore/iris_helpers.py
index ff3fdf2ba7..8d1c676682 100644
--- a/esmvalcore/iris_helpers.py
+++ b/esmvalcore/iris_helpers.py
@@ -93,8 +93,8 @@ def date2num(date, unit, dtype=np.float64):
     This is a custom version of :meth:`cf_units.Unit.date2num` that
     guarantees the correct dtype for the return value.
 
-    Arguments
-    ---------
+    Parameters
+    ----------
     date : :class:`datetime.datetime` or :class:`cftime.datetime`
     unit : :class:`cf_units.Unit`
     dtype : a numpy dtype
@@ -439,8 +439,8 @@ def safe_convert_units(cube: Cube, units: str | Unit) -> Cube:
     that will raise an error if the input cube's
     :attr:`~iris.cube.Cube.standard_name` has been changed.
 
-    Arguments
-    ---------
+    Parameters
+    ----------
     cube:
         Input cube (modified in place).
     units:
diff --git a/esmvalcore/preprocessor/_units.py b/esmvalcore/preprocessor/_units.py
index 8e0c764b78..0ef2c133b6 100644
--- a/esmvalcore/preprocessor/_units.py
+++ b/esmvalcore/preprocessor/_units.py
@@ -49,8 +49,8 @@ def convert_units(cube: Cube, units: str | Unit) -> Cube:
     Note that for precipitation variables, a water density of ``1000 kg m-3``
     is assumed.
 
-    Argumentss
-    ---------
+    Parameters
+    ----------
     cube:
         Input cube (modified in place).
     units:

From 59684877cb6c27e803f4a360f0b219a9cfc8fb82 Mon Sep 17 00:00:00 2001
From: Manuel Schlund <manuel.schlund@dlr.de>
Date: Fri, 18 Oct 2024 10:20:29 +0200
Subject: [PATCH 6/6] Restored original test_convert_units.py

---
 tests/unit/preprocessor/_units/test_convert_units.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tests/unit/preprocessor/_units/test_convert_units.py b/tests/unit/preprocessor/_units/test_convert_units.py
index a56f6f5da0..8fea071943 100644
--- a/tests/unit/preprocessor/_units/test_convert_units.py
+++ b/tests/unit/preprocessor/_units/test_convert_units.py
@@ -1,15 +1,14 @@
 """Unit test for the :func:`esmvalcore.preprocessor._units` function."""
 
+import unittest
+
 import cf_units
 import iris
 import iris.fileformats
 import numpy as np
 
 import tests
-from esmvalcore.preprocessor._units import (
-    accumulate_coordinate,
-    convert_units,
-)
+from esmvalcore.preprocessor._units import accumulate_coordinate, convert_units
 
 
 class TestConvertUnits(tests.Test):
@@ -256,3 +255,7 @@ def test_flux_by_hour(self):
         expected_units = cf_units.Unit("kg")
         self.assertEqual(result.units, expected_units)
         self.assert_array_equal(result.data, expected_data)
+
+
+if __name__ == "__main__":
+    unittest.main()