Repository: ecmwf/pdbufr

Issue:
Message structure not identified correctly
**pdbufr** uses an in memory cache to identify and reuse the message structure as it is processing the messages in a BUFR file. Cache entries are identified by the following header keys and contain all the keys for a given message (structure):

"edition", "masterTableNumber", "numberOfSubsets","unexpandedDescriptors", "delayedDescriptorReplicationFactor"

So if there is already a cache entry for the given message the list of keys are taken from the cache instead of using the key iterator to read them from the message.

The following BUFR file contains 2 messages:

https://get.ecmwf.int/repository/test-data/pdbufr/test-data/message_structure_diff_2.bufr

and according to pdbufr their structure is identical because the value of the keys listed above are the same:

(4, 0, 1, 307096, 22061, 20058, 4024, 13012, 4024,  1, 0)

However, the first message contains more keys as using **bufr_dump -p** confirms it:

This is the end of the first message:

```
#24#timePeriod=-1
depthOfFreshSnow=MISSING
#25#timePeriod=0
```

This is the end of the second message:

```
#20#timePeriod=-1
depthOfFreshSnow=MISSING
#21#timePeriod=0
```

The bottom line is that the message structure identification mechanism does not work correctly in pdbufr and has to be improved.

Discussion:

Patch:
diff --git a/docs/release_notes/version_0.10_updates.rst b/docs/release_notes/version_0.10_updates.rst
index 50f4814..8dcffe7 100644
--- a/docs/release_notes/version_0.10_updates.rst
+++ b/docs/release_notes/version_0.10_updates.rst
@@ -2,6 +2,12 @@
 Version 0.10 Updates
 /////////////////////////
 
+
+Version 0.10.2
+===============
+
+- fixed issue when :func:`read_bufr` incorrectly treated different message structures as if they were identical  (`#49 <https://github.com/ecmwf/pdbufr/issues/49>`_)
+
 Version 0.10.1
 ===============
 
diff --git a/pdbufr/bufr_structure.py b/pdbufr/bufr_structure.py
index 46f59b8..38766a4 100644
--- a/pdbufr/bufr_structure.py
+++ b/pdbufr/bufr_structure.py
@@ -140,15 +140,22 @@ def make_message_uid(message: T.Mapping[str, T.Any]) -> T.Tuple[T.Optional[int],
     else:
         message_uid += tuple(descriptors) + (None,)
 
-    try:
-        delayed_descriptors = message["delayedDescriptorReplicationFactor"]
-    except KeyError:
-        delayed_descriptors = []
+    delayed_keys = (
+        "delayedDescriptorReplicationFactor",
+        "shortDelayedDescriptorReplicationFactor",
+        "extendedDelayedDescriptorReplicationFactor",
+    )
 
-    if isinstance(delayed_descriptors, int):
-        message_uid += (delayed_descriptors,)
-    else:
-        message_uid += tuple(delayed_descriptors)
+    for k in delayed_keys:
+        try:
+            delayed_descriptors = message[k]
+        except KeyError:
+            delayed_descriptors = []
+
+        if isinstance(delayed_descriptors, int):
+            message_uid += (delayed_descriptors, None)
+        else:
+            message_uid += tuple(delayed_descriptors) + (None,)
 
     return message_uid
 


Test patch:
diff --git a/tests/test_20_bufr_structure.py b/tests/test_20_bufr_structure.py
index cc75519..33196e8 100644
--- a/tests/test_20_bufr_structure.py
+++ b/tests/test_20_bufr_structure.py
@@ -145,12 +145,50 @@ def test_filter_keys_cached() -> None:
     assert res == expected_obj
 
     message["delayedDescriptorReplicationFactor"] = 1
+    res = bufr_structure.filter_keys_cached(message, cache)
+    assert len(cache) == 4
+    assert len(res) == 5
 
+    message["delayedDescriptorReplicationFactor"] = (1, 2)
     res = bufr_structure.filter_keys_cached(message, cache)
+    assert len(cache) == 5
+    assert len(res) == 5
 
-    assert len(cache) == 4
+    message["delayedDescriptorReplicationFactor"] = 1
+    res = bufr_structure.filter_keys_cached(message, cache)
+    assert len(cache) == 5
     assert len(res) == 5
 
+    message["shortDelayedDescriptorReplicationFactor"] = 1
+    res = bufr_structure.filter_keys_cached(message, cache)
+    assert len(cache) == 6
+    assert len(res) == 6
+
+    message["shortDelayedDescriptorReplicationFactor"] = (1, 2)
+    res = bufr_structure.filter_keys_cached(message, cache)
+    assert len(cache) == 7
+    assert len(res) == 6
+
+    message["shortDelayedDescriptorReplicationFactor"] = 1
+    res = bufr_structure.filter_keys_cached(message, cache)
+    assert len(cache) == 7
+    assert len(res) == 6
+
+    message["extendedDelayedDescriptorReplicationFactor"] = 1
+    res = bufr_structure.filter_keys_cached(message, cache)
+    assert len(cache) == 8
+    assert len(res) == 7
+
+    message["extendedDelayedDescriptorReplicationFactor"] = (1, 2)
+    res = bufr_structure.filter_keys_cached(message, cache)
+    assert len(cache) == 9
+    assert len(res) == 7
+
+    message["extendedDelayedDescriptorReplicationFactor"] = 1
+    res = bufr_structure.filter_keys_cached(message, cache)
+    assert len(cache) == 9
+    assert len(res) == 7
+
 
 def test_datetime_from_bufr() -> None:
     obs = {"Y": 2020, "M": 3, "D": 18}
diff --git a/tests/test_40_sample_data.py b/tests/test_40_sample_data.py
index 685457e..fd1d7c3 100644
--- a/tests/test_40_sample_data.py
+++ b/tests/test_40_sample_data.py
@@ -1180,7 +1180,7 @@ def test_aircraft_compressed_with_string() -> None:
     assert_frame_equal(res, ref[res.columns])
 
 
-def test_no_value_for_key() -> None:
+def test_message_structure_cache() -> None:
     columns = (
         "stationNumber",
         "timePeriod",
@@ -1188,6 +1188,22 @@ def test_no_value_for_key() -> None:
         "maximumWindGustSpeed",
     )
 
-    f = download_test_data("no_value_timeperiod.bufr")
+    ref = {
+        "stationNumber": {0: 603, 1: 603, 2: 603, 3: 641, 4: 641, 5: 641},
+        "timePeriod": {0: -10, 1: -60, 2: -180, 3: -10, 4: -60, 5: -180},
+        "maximumWindGustDirection": {0: 130, 1: 130, 2: 50, 3: 40, 4: 40, 5: 40},
+        "maximumWindGustSpeed": {
+            0: 2.0,
+            1: 2.4000000000000004,
+            2: 3.5,
+            3: 8.9,
+            4: 8.9,
+            5: 9.5,
+        },
+    }
+
+    f = download_test_data("message_structure_diff_2.bufr")
     res = pdbufr.read_bufr(f, columns=columns)
-    assert len(res) == 3
+
+    assert len(res) == 6
+    assert res.to_dict() == ref