Repository: ecmwf/earthkit-data
Pull request #412 (2024-06-26 12:26:18)

Fix repeated use of build_remapping with a patch
This PR fixes the issue when repeated use of remapping with a patch lost the original remapping. Now this works as expected and in the following code r1 and r2 define the same remapping (and patches).

```python
from earthkit.data.core.order import build_remapping
remapping = {"type": "{type}{number}"}
patch = {"type": {"cf0": "x"}}
r1 = build_remapping(remapping, patch)
r2 = build_remapping(r1)
```

This PR also adds tests for using remapping and patches.


Patch:
diff --git a/src/earthkit/data/core/order.py b/src/earthkit/data/core/order.py
index dbfaf37..5d8f4ae 100644
--- a/src/earthkit/data/core/order.py
+++ b/src/earthkit/data/core/order.py
@@ -72,7 +72,7 @@ def _build_remapping(mapping):
     if mapping is None:
         return Remapping({})
 
-    if not isinstance(mapping, Remapping) and isinstance(mapping, dict):
+    if not isinstance(mapping, (Remapping, Patch)) and isinstance(mapping, dict):
         return Remapping(mapping)
 
     return mapping


Test patch:
diff --git a/tests/core/test_remapping.py b/tests/core/test_remapping.py
new file mode 100644
index 0000000..968c890
--- /dev/null
+++ b/tests/core/test_remapping.py
@@ -0,0 +1,76 @@
+#!/usr/bin/env python3
+
+# (C) Copyright 2020 ECMWF.
+#
+# This software is licensed under the terms of the Apache Licence Version 2.0
+# which can be obtained at http://www.apache.org/licenses/LICENSE-2.0.
+# In applying this licence, ECMWF does not waive the privileges and immunities
+# granted to it by virtue of its status as an intergovernmental organisation
+# nor does it submit to any jurisdiction.
+#
+
+import pytest
+
+from earthkit.data.core.order import build_remapping
+
+data = {"type": "cf", "number": 0, "name": "t2m"}
+
+
+def fn(key):
+    return data[key]
+
+
+@pytest.mark.parametrize(
+    "remapping,expected_values",
+    [
+        ({}, {"type": "cf", "number": 0, "name": "t2m"}),
+        (None, {"type": "cf", "number": 0, "name": "t2m"}),
+        ({"type": "pf"}, {"type": "pf", "number": 0, "name": "t2m"}),
+        ({"type": "{type}{number}"}, {"type": "cf0", "name": "t2m"}),
+        ({"type": "{type}_{number}"}, {"type": "cf_0", "name": "t2m"}),
+        ({"type": lambda: 12}, {"type": 12, "name": "t2m"}),
+    ],
+)
+def test_remapping_dict(remapping, expected_values):
+    r = build_remapping(remapping)
+    _fn = r(fn)
+
+    for k, v in expected_values.items():
+        assert _fn(k) == v
+
+
+def test_remapping_repeated():
+    remapping = {"type": "{type}{number}"}
+    r = build_remapping(remapping)
+    r = build_remapping(r)
+    _fn = r(fn)
+
+    assert _fn("type") == "cf0"
+
+
+@pytest.mark.parametrize(
+    "remapping,patch,expected_values",
+    [
+        ({}, {"type": {"cf": "pf"}, "number": 12, "name": None}, {"type": "pf", "number": 12, "name": None}),
+        ({"type": "{type}{number}"}, {"type": {"cf0": "x"}}, {"type": "x", "number": 0, "name": "t2m"}),
+        ({}, {"type": lambda x: "x"}, {"type": "x", "number": 0, "name": "t2m"}),
+        ({}, {"type": True}, {"type": True, "number": 0, "name": "t2m"}),
+        (None, {"type": True}, {"type": True, "number": 0, "name": "t2m"}),
+    ],
+)
+def test_remapping_patch(remapping, patch, expected_values):
+    r = build_remapping(remapping, patch)
+    _fn = r(fn)
+
+    for k, v in expected_values.items():
+        assert _fn(k) == v
+
+
+def test_remapping_patch_repeated():
+    remapping = {"type": "{type}{number}"}
+    patch = {"type": {"cf0": "x"}}
+    r = build_remapping(remapping, patch)
+    r = build_remapping(r)
+    _fn = r(fn)
+
+    assert _fn("type") == "x"