From 9d926f52500d2916c4b84eda26b06c36baabe3d6 Mon Sep 17 00:00:00 2001 From: Romain Beucher Date: Wed, 8 Jul 2026 10:09:17 +1000 Subject: [PATCH 1/6] Remove kelvin_to_celsius from tob mapping --- src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json b/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json index 91c9a877..52beadcc 100644 --- a/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json +++ b/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json @@ -5459,14 +5459,9 @@ ], "calculation": { "type": "formula", - "operation": "kelvin_to_celsius", + "operation": "ocean_floor", "operands": [ - { - "operation": "ocean_floor", - "operands": [ - "pot_temp" - ] - } + "pot_temp" ] }, "CF standard Name": "sea_water_potential_temperature_at_sea_floor", From 18d1a52b24004756c53c76d874342af947dd8e7c Mon Sep 17 00:00:00 2001 From: Romain Beucher Date: Wed, 8 Jul 2026 10:14:06 +1000 Subject: [PATCH 2/6] Restore thetao Kelvin conversion with fallback --- src/access_moppy/derivations/__init__.py | 22 ++++++++++++++++- .../mappings/ACCESS-ESM1-6_mappings.json | 7 ++++-- tests/unit/test_derivations.py | 24 +++++++++++++++++++ 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/access_moppy/derivations/__init__.py b/src/access_moppy/derivations/__init__.py index a1b5d977..f63b94ca 100644 --- a/src/access_moppy/derivations/__init__.py +++ b/src/access_moppy/derivations/__init__.py @@ -71,7 +71,7 @@ "power": lambda a, b: a**b, "sum": lambda x, **kwargs: x.sum(**kwargs), "mean": lambda *args: sum(args) / len(args), - "kelvin_to_celsius": lambda x: x - 273.15, + "kelvin_to_celsius": lambda x: _kelvin_to_celsius(x), "celsius_to_kelvin": lambda x: x + 273.15, "cli_level_to_height": cli_level_to_height, "clw_level_to_height": clw_level_to_height, @@ -128,6 +128,26 @@ } +def _kelvin_to_celsius(value): + """Convert Kelvin to Celsius with a fallback for mislabeled Celsius-scale data.""" + attrs = getattr(value, "attrs", {}) or {} + units = str(attrs.get("units", "")).strip().lower() + + if "celsius" in units or "degc" in units or units in {"c", "deg c"}: + return value + + if units in {"k", "kelvin"}: + valid_range = attrs.get("valid_range") + try: + if valid_range is not None and len(valid_range) >= 1: + if float(valid_range[0]) < 100.0: + return value + except Exception: + pass + + return value - 273.15 + + def evaluate_expression(expr, context): if isinstance(expr, dict): if "literal" in expr: diff --git a/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json b/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json index 52beadcc..3ab881f9 100644 --- a/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json +++ b/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json @@ -5382,8 +5382,11 @@ "pot_temp" ], "calculation": { - "type": "direct", - "formula": "pot_temp" + "type": "formula", + "operation": "kelvin_to_celsius", + "operands": [ + "pot_temp" + ] }, "CF standard Name": "", "model_variable_units": { diff --git a/tests/unit/test_derivations.py b/tests/unit/test_derivations.py index 07a86b18..3ef0cbfe 100644 --- a/tests/unit/test_derivations.py +++ b/tests/unit/test_derivations.py @@ -62,6 +62,30 @@ def test_optional_used_as_kwarg_in_formula(self): result = evaluate_expression(expr, ctx) np.testing.assert_allclose(result.values, 2.0) + def test_kelvin_to_celsius_converts_kelvin_values(self): + """kelvin_to_celsius should convert canonical Kelvin metadata.""" + da = xr.DataArray( + np.array([300.0], dtype=np.float32), + attrs={"units": "K", "valid_range": [250.0, 350.0]}, + ) + result = evaluate_expression( + {"operation": "kelvin_to_celsius", "operands": ["temp"]}, + {"temp": da}, + ) + assert float(result.values[0]) == pytest.approx(26.85, abs=1e-3) + + def test_kelvin_to_celsius_skips_celsius_scale_metadata(self): + """kelvin_to_celsius should not offset known Celsius-scale mislabeled data.""" + da = xr.DataArray( + np.array([10.0], dtype=np.float32), + attrs={"units": "K", "valid_range": [-10.0, 500.0]}, + ) + result = evaluate_expression( + {"operation": "kelvin_to_celsius", "operands": ["temp"]}, + {"temp": da}, + ) + assert float(result.values[0]) == pytest.approx(10.0) + class TestCalculateMonthlyMinimum: """Tests for calculate_monthly_minimum() — covers decode_cf and ME frequency fix.""" From 190b5460cb42cda03567ba4223e0981ce2aee61d Mon Sep 17 00:00:00 2001 From: Romain Beucher Date: Wed, 8 Jul 2026 10:14:57 +1000 Subject: [PATCH 3/6] Restore tob Kelvin conversion --- .../mappings/ACCESS-ESM1-6_mappings.json | 9 +++++++-- tests/unit/test_derivations.py | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json b/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json index 3ab881f9..21cd8296 100644 --- a/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json +++ b/src/access_moppy/mappings/ACCESS-ESM1-6_mappings.json @@ -5462,9 +5462,14 @@ ], "calculation": { "type": "formula", - "operation": "ocean_floor", + "operation": "kelvin_to_celsius", "operands": [ - "pot_temp" + { + "operation": "ocean_floor", + "operands": [ + "pot_temp" + ] + } ] }, "CF standard Name": "sea_water_potential_temperature_at_sea_floor", diff --git a/tests/unit/test_derivations.py b/tests/unit/test_derivations.py index 3ef0cbfe..03d0b9a6 100644 --- a/tests/unit/test_derivations.py +++ b/tests/unit/test_derivations.py @@ -86,6 +86,25 @@ def test_kelvin_to_celsius_skips_celsius_scale_metadata(self): ) assert float(result.values[0]) == pytest.approx(10.0) + def test_ocean_floor_then_kelvin_to_celsius(self): + """The tob-style chain should convert the floor value from Kelvin to Celsius.""" + data = xr.DataArray( + np.array([[300.0, 280.0], [295.0, np.nan], [290.0, np.nan]], dtype=np.float32), + dims=["st_ocean", "xt_ocean"], + attrs={"units": "K", "valid_range": [250.0, 350.0]}, + ) + result = evaluate_expression( + { + "operation": "kelvin_to_celsius", + "operands": [ + {"operation": "ocean_floor", "operands": ["temp"]}, + ], + }, + {"temp": data}, + ) + assert float(result.isel(xt_ocean=0).values) == pytest.approx(16.85, abs=1e-3) + assert float(result.isel(xt_ocean=1).values) == pytest.approx(6.85, abs=1e-3) + class TestCalculateMonthlyMinimum: """Tests for calculate_monthly_minimum() — covers decode_cf and ME frequency fix.""" From 317a2b406d7608f87ec4cb49ac53e518c1ff93a2 Mon Sep 17 00:00:00 2001 From: Romain Beucher Date: Wed, 8 Jul 2026 10:19:54 +1000 Subject: [PATCH 4/6] Format array initialization in test_ocean_floor_then_kelvin_to_celsius for better readability --- tests/unit/test_derivations.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_derivations.py b/tests/unit/test_derivations.py index 03d0b9a6..c0d2824e 100644 --- a/tests/unit/test_derivations.py +++ b/tests/unit/test_derivations.py @@ -89,7 +89,9 @@ def test_kelvin_to_celsius_skips_celsius_scale_metadata(self): def test_ocean_floor_then_kelvin_to_celsius(self): """The tob-style chain should convert the floor value from Kelvin to Celsius.""" data = xr.DataArray( - np.array([[300.0, 280.0], [295.0, np.nan], [290.0, np.nan]], dtype=np.float32), + np.array( + [[300.0, 280.0], [295.0, np.nan], [290.0, np.nan]], dtype=np.float32 + ), dims=["st_ocean", "xt_ocean"], attrs={"units": "K", "valid_range": [250.0, 350.0]}, ) From c809d280fa187e73dd9720dab16bf1b8fed14ba8 Mon Sep 17 00:00:00 2001 From: Romain Beucher Date: Wed, 8 Jul 2026 10:22:17 +1000 Subject: [PATCH 5/6] Add Kelvin fallback coverage tests --- tests/unit/test_derivations.py | 53 ++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/unit/test_derivations.py b/tests/unit/test_derivations.py index c0d2824e..e2c13d66 100644 --- a/tests/unit/test_derivations.py +++ b/tests/unit/test_derivations.py @@ -86,6 +86,47 @@ def test_kelvin_to_celsius_skips_celsius_scale_metadata(self): ) assert float(result.values[0]) == pytest.approx(10.0) + def test_kelvin_to_celsius_passes_through_celsius_units(self): + """kelvin_to_celsius should leave Celsius-labelled values unchanged.""" + da = xr.DataArray(np.array([12.5], dtype=np.float32), attrs={"units": "degC"}) + result = evaluate_expression( + {"operation": "kelvin_to_celsius", "operands": ["temp"]}, + {"temp": da}, + ) + assert float(result.values[0]) == pytest.approx(12.5) + + def test_kelvin_to_celsius_converts_non_kelvin_units(self): + """Non-Celsius, non-Kelvin units should still apply the Kelvin offset.""" + da = xr.DataArray(np.array([273.15], dtype=np.float32), attrs={"units": "m"}) + result = evaluate_expression( + {"operation": "kelvin_to_celsius", "operands": ["temp"]}, + {"temp": da}, + ) + assert float(result.values[0]) == pytest.approx(0.0, abs=1e-3) + + def test_kelvin_to_celsius_ignores_malformed_valid_range(self): + """Malformed valid_range metadata should fall back to the Kelvin offset.""" + + class BadRange: + def __len__(self): + raise RuntimeError("bad valid_range") + + da = xr.DataArray(np.array([280.0], dtype=np.float32), attrs={"units": "K", "valid_range": BadRange()}) + result = evaluate_expression( + {"operation": "kelvin_to_celsius", "operands": ["temp"]}, + {"temp": da}, + ) + assert float(result.values[0]) == pytest.approx(6.85, abs=1e-3) + + def test_kelvin_to_celsius_without_valid_range_uses_offset(self): + """Kelvin units without valid_range metadata should still convert.""" + da = xr.DataArray(np.array([280.0], dtype=np.float32), attrs={"units": "K"}) + result = evaluate_expression( + {"operation": "kelvin_to_celsius", "operands": ["temp"]}, + {"temp": da}, + ) + assert float(result.values[0]) == pytest.approx(6.85, abs=1e-3) + def test_ocean_floor_then_kelvin_to_celsius(self): """The tob-style chain should convert the floor value from Kelvin to Celsius.""" data = xr.DataArray( @@ -107,6 +148,18 @@ def test_ocean_floor_then_kelvin_to_celsius(self): assert float(result.isel(xt_ocean=0).values) == pytest.approx(16.85, abs=1e-3) assert float(result.isel(xt_ocean=1).values) == pytest.approx(6.85, abs=1e-3) + def test_list_expression_is_evaluated_recursively(self): + """evaluate_expression should recurse through list expressions.""" + ctx = self._make_context() + result = evaluate_expression(["var1", {"literal": 3}], ctx) + assert result[0] is ctx["var1"] + assert result[1] == 3 + + def test_unsupported_expression_raises(self): + """Unsupported expressions should raise a ValueError.""" + with pytest.raises(ValueError, match="Unsupported expression"): + evaluate_expression(object(), {}) + class TestCalculateMonthlyMinimum: """Tests for calculate_monthly_minimum() — covers decode_cf and ME frequency fix.""" From 959e86ce24371227a53d821f400a719335d66e6c Mon Sep 17 00:00:00 2001 From: Romain Beucher Date: Wed, 8 Jul 2026 10:25:11 +1000 Subject: [PATCH 6/6] pre-commit --- tests/unit/test_derivations.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_derivations.py b/tests/unit/test_derivations.py index e2c13d66..e8336d43 100644 --- a/tests/unit/test_derivations.py +++ b/tests/unit/test_derivations.py @@ -111,7 +111,10 @@ class BadRange: def __len__(self): raise RuntimeError("bad valid_range") - da = xr.DataArray(np.array([280.0], dtype=np.float32), attrs={"units": "K", "valid_range": BadRange()}) + da = xr.DataArray( + np.array([280.0], dtype=np.float32), + attrs={"units": "K", "valid_range": BadRange()}, + ) result = evaluate_expression( {"operation": "kelvin_to_celsius", "operands": ["temp"]}, {"temp": da},