Merge pull request #49 from wmo-im/rory-s4-fix

Correct section 3 and 4 mappings + more

.github/workflows/tests.yml

@@ -7,13 +7,13 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: [3.7, 3.8, 3.9]
+        python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"]
     env:
       BUFR_ORIGINATING_CENTRE: 123
       BUFR_ORIGINATING_SUBCENTRE: 123
     steps:
-    - uses: actions/checkout@v2
-    - uses: actions/setup-python@v2
+    - uses: actions/checkout@v4
+    - uses: actions/setup-python@v5
       name: Setup Python ${{ matrix.python-version }}
       with:
         python-version: ${{ matrix.python-version }}

.gitignore

@@ -42,8 +42,14 @@ nosetests.xml
 logs
 .vscode/
 .vscode/settings.json
+# Pytest cache files
+tests/__pycache__/
 # Ignore decoded CSV files
 decoded_*.csv
+# Ignore extra mapping files in data folders generated by synop2bufr
+data/**/*.json
+# Ignore bash scripts in data folder
+data/*.sh
 
 # pycharm
 .idea

README.md

@@ -17,14 +17,7 @@ Dependencies are listed in [requirements.txt](https://github.com/wmo-im/synop2bu
 
 Before using synop2bufr, we highly encourage you to set the `BUFR_ORIGINATING_CENTRE` and `BUFR_ORIGINATING_SUBCENTRE` environment variables. These variables are used to specify the originating centre and subcentre of the SYNOP messages. **Without these set, they will default to missing (255).**
 
-It is recommended that you set these environment variables in the Dockerfile, by editing the following lines with your originating centre and subcentre values:
-
-```bash
-ENV BUFR_ORIGINATING_CENTRE=<centre_value>
-ENV BUFR_ORIGINATING_SUBCENTRE=<subcentre_value>
-```
-
-Alternatively, you can set these environment variables in your shell if you want to run synop2bufr on your local machine. Here's how you can do it in a Bash shell:
+You can set these environment variables in your shell if you want to run synop2bufr on your local machine. Here's how you can do it in a Bash shell:
 
 ```bash
 export BUFR_ORIGINATING_CENTRE=<centre_value>
@@ -36,8 +29,11 @@ export BUFR_ORIGINATING_SUBCENTRE=<subcentre_value>
 To run synop2bufr from a Docker container:
 
 ```console
-docker build -t synop2bufr:local .
-docker run -it -v ${pwd}:/local synop2bufr
+docker run -it -v /$(pwd):/local wmoim/dim_eccodes_baseimage:2.34.0 bash
+apt-get update && apt-get install -y git
+cd /local
+python3 setup.py install
+synop2bufr --help
 ```
 
 Example data can be found in `data` directory, with the corresponding reference BUFR4 in `data/bufr`.

docs/source/installation.rst

@@ -45,21 +45,20 @@ Alternatively, synop2bufr can be installed from source. First clone the reposito
    git clone https://github.com/wmo-im/synop2bufr.git
    cd synop2bufr
 
-If running in a Docker environment, build the Docker image and run the container:
+You can then run synop2bufr from an ecCodes base image as follows:
 
 .. code-block:: bash
 
-   docker build -t synop2bufr .
-   docker run -it -v ${pwd}:/app synop2bufr
-   cd /app
+   docker run -it -v /$(pwd):/local wmoim/dim_eccodes_baseimage:2.34.0 bash
+   apt-get update && apt-get install -y git
+   cd /local
+   python3 setup.py install
+   synop2bufr --help
 
 The above step can be skipped if not using Docker. If not using Docker the module and dependencies needs to be installed:
 
 .. code-block:: bash
-
-   pip3 install -r requirements.txt
-   pip3 install --no-cache-dir https://github.com/wmo-im/csv2bufr/archive/refs/tags/v0.3.1.zip
-   pip3 install --no-cache-dir https://github.com/wmo-im/pymetdecoder/archive/refs/tags/v0.1.0.zip
+   
    python3 setup.py install
    synop2bufr --help
 

requirements.txt

@@ -1,4 +1,3 @@
-attrs==22.2.0
-click==8.1.3
-numpy==1.21.6
-csv2bufr
+click
+pymetdecoder-wmo
+csv2bufr @ git+https://github.com/wmo-im/csv2bufr.git

setup.py

@@ -23,6 +23,7 @@
 import os
 import re
 from setuptools import Command, find_packages, setup
+import subprocess
 
 
 class PyTest(Command):
@@ -73,6 +74,8 @@ def get_package_version():
 if (os.path.exists('MANIFEST')):
     os.unlink('MANIFEST')
 
+# Install dependencies not on PyPI
+subprocess.check_call("pip install 'csv2bufr @ git+https://github.com/wmo-im/csv2bufr.git'", shell=True) # noqa
 
 setup(
     name='synop2bufr',

synop2bufr/__init__.py

@@ -1014,7 +1014,11 @@ def rad_convert(rad, time):
             # The time period is expected to be in hours
             output['ps3_time_period'] = -1 * decoded['precipitation_s3']['time_before_obs']['value']  # noqa
         except Exception:
-            output['ps3_time_period'] = None
+            # Regional manual (1/12.11, 2/12.12, 3/12.10, etc.) states that
+            # the precipitation time period is 3 hours,
+            # or another period required for regional exchange.
+            # This means that if tR is not given, it is assumed to be 3 hours.
+            output['ps3_time_period'] = -3
 
     # Precipitation indicator iR is needed to determine whether the
     # section 1 and section 3 precipitation groups are missing because there
@@ -1205,10 +1209,7 @@ def extract_individual_synop(data: str) -> list:
 
     :returns: `list` of messages
     """
-
-    # Check for abbreviated header line TTAAii etc.
-
-    # Now split based as section 0 of synop, beginning AAXX YYGGi_w
+    # Split string based on section 0 of FM-12, beginning with AAXX
     start_position = data.find("AAXX")
 
     # Start position is -1 if AAXX is not present in the message
@@ -1217,20 +1218,31 @@ def extract_individual_synop(data: str) -> list:
             "Invalid SYNOP message: AAXX could not be found."
         )
 
-    data = re.split('(AAXX [0-9]{5})', data[start_position:])
+    # Split the string by AAXX YYGGiw
+    data = re.split(r'(AAXX\s+[0-9]{5})', data[start_position:])
+
+    # Check if the beginning of the message (e.g. ZCZC 123 etc.)
+    # that we're about to throw away (data[0]) also contains AAXX.
+    # If this is true, there must be a typo present at the AAXX YYGGiw
+    # part and thus we can't process the message.
+    if "AAXX" in data[0]:
+        raise ValueError((
+            f"The following SYNOP message is invalid: {data[0]}"
+            " Please check again for typos."
+        ))
+
     data = data[1:]  # Drop first null element
     # Iterate over messages processing
     messages = []
     for d in data:
         if "AAXX" in d:
             s0 = d
         else:
-            if not d.__contains__("="):
-                LOGGER.error((
+            if "=" not in d:
+                raise ValueError((
                     "Delimiters (=) are not present in the string,"
                     " thus unable to identify separate SYNOP reports."
-                    ))  # noqa
-                raise ValueError
+                ))
 
             d = re.sub(r"\n+", " ", d)
             d = re.sub(r"\x03", "", d)
@@ -1478,6 +1490,7 @@ def transform(data: str, metadata: str, year: int,
                 wsi_series, wsi_issuer, wsi_issue_number, wsi_local = wsi.split("-")   # noqa
 
                 # get other required metadata
+                station_name = metadata_dict[wsi]["station_name"]
                 latitude = metadata_dict[wsi]["latitude"]
                 longitude = metadata_dict[wsi]["longitude"]
                 station_height = metadata_dict[wsi]["elevation"]
@@ -1488,6 +1501,7 @@ def transform(data: str, metadata: str, year: int,
                 msg['_wsi_issuer'] = wsi_issuer
                 msg['_wsi_issue_number'] = wsi_issue_number
                 msg['_wsi_local'] = wsi_local
+                msg['_station_name'] = station_name
                 msg['_latitude'] = latitude
                 msg['_longitude'] = longitude
                 msg['_station_height'] = station_height
@@ -1541,9 +1555,25 @@ def transform(data: str, metadata: str, year: int,
                     # Stop duplicated warnings
                     can_var_warning_be_displayed = False
 
-                # Now we need to add the mappings for the cloud groups
+                # Define a new method which handles the updating of
+                # the mapping file with section 3 and 4 cloud data
+                def update_data_mapping(mapping: list, update: dict):
+                    match = False
+                    for idx in range(len(mapping)):
+                        if mapping[idx]['eccodes_key'] == update['eccodes_key']:  # noqa
+                            match = True
+                            break
+                    if match:
+                        mapping[idx] = update
+                    else:
+                        mapping.append(update)
+                    return mapping
+
+                # Now we add the mappings for the cloud groups
                 # of section 3 and 4
                 try:
+
+                    # Now add the rest of the mappings for section 3 clouds
                     for idx in range(num_s3_clouds):
                         # Build the dictionary of mappings for section 3
                         # group 8NsChshs
@@ -1556,12 +1586,10 @@ def transform(data: str, metadata: str, year: int,
                         # - verticalSignificance: used 7 times (for N,
                         # low-high cloud amount, low-high cloud drift)
                         s3_mappings = [
-                            {"eccodes_key": (
-                                f"#{idx+8}"
-                                "#verticalSignificanceSurfaceObservations"
-                            ),
+                            {"eccodes_key":
+                                f"#{idx+6}#verticalSignificanceSurfaceObservations",  # noqa
                                 "value": f"data:vs_s3_{idx+1}"},
-                            {"eccodes_key": f"#{idx+3}#cloudAmount",
+                            {"eccodes_key": f"#{idx+2}#cloudAmount",
                                 "value": f"data:cloud_amount_s3_{idx+1}",
                                 "valid_min": "const:0",
                                 "valid_max": "const:8"},
@@ -1571,8 +1599,10 @@ def transform(data: str, metadata: str, year: int,
                                 "value": f"data:cloud_height_s3_{idx+1}"}
                         ]
                         for m in s3_mappings:
-                            mapping.update(m)
+                            mapping['data'] = update_data_mapping(
+                                mapping=mapping['data'], update=m)
 
+                    # Now add the rest of the mappings for section 4 clouds
                     for idx in range(num_s4_clouds):
                         # Based upon the station height metadata, the
                         # value of vertical significance for section 4
@@ -1592,13 +1622,11 @@ def transform(data: str, metadata: str, year: int,
                         # NOTE: Some of the ecCodes keys are used in
                         # the above, so we must add 'num_s3_clouds'
                         s4_mappings = [
-                            {"eccodes_key": (
-                                f"#{idx+num_s3_clouds+8}"
-                                "#verticalSignificanceSurfaceObservations"
-                            ),
+                            {"eccodes_key":
+                                f"#{idx+num_s3_clouds+6}#verticalSignificanceSurfaceObservations",  # noqa
                                 "value": f"const:{vs_s4}"},
                             {"eccodes_key":
-                                f"#{idx+num_s3_clouds+3}#cloudAmount",
+                                f"#{idx+num_s3_clouds+2}#cloudAmount",
                                 "value": f"data:cloud_amount_s4_{idx+1}",
                                 "valid_min": "const:0",
                                 "valid_max": "const:8"},
@@ -1613,7 +1641,8 @@ def transform(data: str, metadata: str, year: int,
                                 "value": f"data:cloud_top_s4_{idx+1}"}
                         ]
                         for m in s4_mappings:
-                            mapping.update(m)
+                            mapping['data'] = update_data_mapping(
+                                mapping=mapping['data'], update=m)
                 except Exception as e:
                     LOGGER.error(e)
                     LOGGER.error(f"Missing station height for station {tsi}")
@@ -1628,8 +1657,7 @@ def transform(data: str, metadata: str, year: int,
                 unexpanded_descriptors = [301150, bufr_template]
                 short_delayed_replications = []
                 # update replications
-                delayed_replications = [max(1, num_s3_clouds),
-                                        max(1, num_s4_clouds)]
+                delayed_replications = [num_s3_clouds, num_s4_clouds]
                 extended_delayed_replications = []
                 table_version = 37
                 try:

synop2bufr/resources/synop-mappings-307080.json

@@ -11,10 +11,10 @@
         "id": "b3abe03b-e502-48c5-a225-133b189207ee"
     },
     "inputShortDelayedDescriptorReplicationFactor": [],
-    "inputDelayedDescriptorReplicationFactor": [1,1],
+    "inputDelayedDescriptorReplicationFactor": [0,0],
     "inputExtendedDelayedDescriptorReplicationFactor": [],
     "number_header_rows": 1,
-    "names_on_row": 1,
+    "column_names_row": 1,
     "wigos_station_identifier": "data:Station_ID",
     "header":[
         {"eccodes_key": "edition", "value": "const:4"},
@@ -46,7 +46,7 @@
         {"eccodes_key": "#1#heightOfBarometerAboveMeanSeaLevel", "value":"data:_barometer_height", "valid_min": "const:-400.0", "valid_max": "const:12707.1"},
         {"eccodes_key": "#1#blockNumber", "value": "data:block_no", "valid_min": "const:0", "valid_max": "const:127"},
         {"eccodes_key": "#1#stationNumber", "value": "data:station_no", "valid_min": "const:0", "valid_max": "const:1023"},
-        {"eccodes_key": "#1#stationOrSiteName", "value": "data:station_id"},
+        {"eccodes_key": "#1#stationOrSiteName", "value": "data:_station_name"},
         {"eccodes_key": "#1#stationType", "value": "data:WMO_station_type", "valid_min": "const:0", "valid_max": "const:3"},
         {"eccodes_key": "#1#year", "value": "data:year", "valid_min": "const:0", "valid_max": "const:4095"},
         {"eccodes_key": "#1#month", "value": "data:month", "valid_min": "const:0", "valid_max": "const:15"},
@@ -72,15 +72,12 @@
         {"eccodes_key": "#1#cloudType", "value": "data:low_cloud_type", "valid_min": "const:0", "valid_max": "const:63"},
         {"eccodes_key": "#2#cloudType", "value": "data:middle_cloud_type", "valid_min": "const:0", "valid_max": "const:63"},
         {"eccodes_key": "#3#cloudType", "value": "data:high_cloud_type", "valid_min": "const:0", "valid_max": "const:63"},
-        {"eccodes_key": "#2#verticalSignificanceSurfaceObservations", "value": "const:7", "valid_min": "const:0", "valid_max": "const:63"},
-        {"eccodes_key": "#3#verticalSignificanceSurfaceObservations", "value": "const:8", "valid_min": "const:0", "valid_max": "const:63"},
-        {"eccodes_key": "#4#verticalSignificanceSurfaceObservations", "value": "const:9", "valid_min": "const:0", "valid_max": "const:63"},
         {"eccodes_key": "#1#trueDirectionFromWhichAPhenomenonOrCloudsAreMovingOrInWhichTheyAreObserved", "value": "data:low_cloud_drift_direction", "valid_min": "const:0.0", "valid_max": "const:360"},
         {"eccodes_key": "#2#trueDirectionFromWhichAPhenomenonOrCloudsAreMovingOrInWhichTheyAreObserved", "value": "data:middle_cloud_drift_direction", "valid_min": "const:0.0", "valid_max": "const:360"},
         {"eccodes_key": "#3#trueDirectionFromWhichAPhenomenonOrCloudsAreMovingOrInWhichTheyAreObserved", "value": "data:high_cloud_drift_direction", "valid_min": "const:0.0", "valid_max": "const:360"},
-        {"eccodes_key": "#5#verticalSignificanceSurfaceObservations", "value": "const:7", "valid_min": "const:0", "valid_max": "const:63"},
-        {"eccodes_key": "#6#verticalSignificanceSurfaceObservations", "value": "const:8", "valid_min": "const:0", "valid_max": "const:63"},
-        {"eccodes_key": "#7#verticalSignificanceSurfaceObservations", "value": "const:9", "valid_min": "const:0", "valid_max": "const:63"},
+        {"eccodes_key": "#2#verticalSignificanceSurfaceObservations", "value": "const:7", "valid_min": "const:0", "valid_max": "const:63"},
+        {"eccodes_key": "#3#verticalSignificanceSurfaceObservations", "value": "const:8", "valid_min": "const:0", "valid_max": "const:63"},
+        {"eccodes_key": "#4#verticalSignificanceSurfaceObservations", "value": "const:9", "valid_min": "const:0", "valid_max": "const:63"},
         {"eccodes_key": "#1#stateOfGround", "value": "data:ground_state", "valid_min": "const:0", "valid_max": "const:31"},
         {"eccodes_key": "#1#totalSnowDepth", "value": "data:snow_depth", "valid_min": "const:0.0", "valid_max": "const:25"},
         {"eccodes_key": "#1#groundMinimumTemperaturePast12Hours", "value": "data:ground_temperature", "valid_min": "const:0.0", "valid_max": "const:655.35"},