Small review changes.

docs/ref/release_notes.rst

@@ -19,15 +19,15 @@ Features
 * `@pp-mo <https://github.com/pp-mo>`_ expanded the use of the "GRIB_PARAM"
   attributes to GRIB1 loading, and document it more thoroughly.
   `(ISSUE#330) <https://github.com/SciTools/iris-grib/issues/330>`_,
-  `(PR#442) <https://github.com/SciTools/iris-grib/pull/402>`_
+  `(PR#402) <https://github.com/SciTools/iris-grib/pull/402>`_
 
 Documentation
 ^^^^^^^^^^^^^
 * `@pp-mo <https://github.com/pp-mo>`_ reworked the main docs page to :
   headline basic load + save with Iris, rather than lower-level functions;
   better explain load-pairs and save-pairs usage; make all usage examples into
   doctests.
-  `(PR#398) <https://github.com/SciTools/iris-grib/pull/398>`_
+  `(ISSUE#398) <https://github.com/SciTools/iris-grib/issues/398>`_
 
 
 What's new in iris-grib v0.19.1

iris_grib/grib_phenom_translation/_gribcode.py

@@ -29,24 +29,28 @@ def _invalid_nargs(args):
     raise ValueError(msg)
 
 
+# Regexp to extract four integers from a string:
+# - for four times ...
+# - match any non-digits (including none) and discard
+# - then match any digits (including none), and return as a "group"
 _RE_PARSE_FOURNUMS = re.compile(4 * r'[^\d]*(\d*)')
 
 
-def _fournums_from_gribcode_string(edcn_string):
-    parsed_ok = False
-    nums_match = _RE_PARSE_FOURNUMS.match(edcn_string).groups()
-    if nums_match is not None:
-        try:
-            nums = [int(grp) for grp in nums_match]
-            parsed_ok = True
-        except ValueError:
-            pass
+def _fournums_from_gribcode_string(grib_param_string):
+    parsed_ok = True
+    # get the numbers..
+    match_groups = _RE_PARSE_FOURNUMS.match(grib_param_string).groups()
+    # N.B. always produces 4 "strings of digits", but some can be empty
+    try:
+        nums = [int(grp) for grp in match_groups]
+    except ValueError:
+        parsed_ok = False
 
     if not parsed_ok:
         msg = ('Invalid argument for GRIBCode creation, '
                '"GRIBCode({!r})" : '
                'requires 4 numbers, separated by non-numerals.')
-        raise ValueError(msg.format(edcn_string))
+        raise ValueError(msg.format(grib_param_string))
 
     return nums
 

iris_grib/tests/unit/grib_phenom_translation/test_grib_phenom_translation.py

@@ -154,7 +154,6 @@ class TestGRIBcode(tests.IrisTest):
     # GRIBCode is basically a namedtuple, so not all behaviour needs testing.
     # However, creation is a bit special so exercise all those cases.
 
-    # TODO: (still) test new + separate GRIB1 behaviour also
     # TODO: convert to pytest + replace duplications with parameterisation
     #   (mostly grib1/grib2, but also in one case str/repr)
     def test_create_from_keys__grib2(self):
@@ -170,7 +169,6 @@ def test_create_from_keys__grib2(self):
         self.assertEqual(gribcode.number, 199)
 
     def test_create_from_keys__grib1(self):
-        # TODO: when we get pytest, combine with above using parameters
         gribcode = GRIBCode(
             edition=1,
             table_version=7,
@@ -190,7 +188,6 @@ def test_create_from_args__grib2(self):
         self.assertEqual(gribcode.number, 99)
 
     def test_create_from_args__grib1(self):
-        # TODO: when we get pytest, combine with above using parameters
         gribcode = GRIBCode(1, 3, 12, 99)
         self.assertEqual(gribcode.edition, 1)
         self.assertEqual(gribcode.table_version, 3)
@@ -204,12 +201,10 @@ def check_create_is_copy(self, edition):
         self.assertIsNot(gribcode1, gribcode2)
 
     def test_create_is_copy__grib1(self):
-        # TODO: when we get pytest, combine with above using parameters
         self.check_create_is_copy(edition=1)
 
     def test_create_is_copy__grib2(self):
-        # TODO: when we get pytest, combine with above using parameters
-        self.check_create_is_copy(edition=1)
+        self.check_create_is_copy(edition=2)
 
     def check_create_from_gribcode(self, edition):
         gribcode1 = GRIBCode((edition, 3, 2, 1))
@@ -220,11 +215,9 @@ def check_create_from_gribcode(self, edition):
         self.assertIsNot(gribcode1, gribcode2)
 
     def test_create_from_gribcode__grib1(self):
-        # TODO: when we get pytest, combine with above using parameters
         self.check_create_from_gribcode(edition=1)
 
     def test_create_from_gribcode__grib2(self):
-        # TODO: when we get pytest, combine with above using parameters
         self.check_create_from_gribcode(edition=2)
 
     def check_create_from_string(self, edition):
@@ -233,11 +226,9 @@ def check_create_from_string(self, edition):
         self.assertEqual(gribcode, GRIBCode(edition, 34, 5, 678))
 
     def test_create_from_string__grib1(self):
-        # TODO: when we get pytest, combine with above using parameters
         self.check_create_from_string(edition=1)
 
     def test_create_from_string__grib2(self):
-        # TODO: when we get pytest, combine with above using parameters
         self.check_create_from_string(edition=2)
 
     def check_create_from_own_string(self, string_function, edition):
@@ -353,16 +344,22 @@ def test__str__grib2(self):
 
     def check__repr__(self, edition):
         result = repr(GRIBCode(edition, 17, 3, 123))
-        expected = (
-            f"GRIBCode(edition={edition}, discipline=17, "
-            "category=3, number=123)"
-        )
+        if edition == 1:
+            expected = (
+                "GRIBCode(edition=1, table_version=17, "
+                "centre_number=3, number=123)"
+            )
+        elif edition == 2:
+            expected = (
+                "GRIBCode(edition=2, discipline=17, "
+                "category=3, number=123)"
+            )
         self.assertEqual(result, expected)
 
-    def check__repr__grib1(self):
+    def test__repr__grib1(self):
         self.check__repr__(edition=1)
 
-    def check__repr__grib2(self):
+    def test__repr__grib2(self):
         self.check__repr__(edition=2)
 
     def test_bad_content__str_repr__badedition(self):