Repository: NCAS-CMS/cf-python

Issue:
Setting both `relaxed_identities=True`and `ncvar_identities=True` in` cf.aggregate` doesn't work.
As of v3.12.0, `cf.aggregate` with `relaxed_identities=True` raises an obscure exception when `ncvar_identities=True` is also set. This is not necessary. The following should aggregate nicely:
```python
>>> import cf
>>> f = cf.example_field(0)
>>> cf.aggregate([f[:2], f[2:]], relaxed_identities=True, ncvar_identities=True) 
Traceback (most recent call last):
    ...
VaueError: 'relaxed' and 'nc_only' parameters cannot both be True
```



Discussion:

Patch:
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
deleted file mode 100644
index 6573a37a7..000000000
--- a/.github/pull_request_template.md
+++ /dev/null
@@ -1,5 +0,0 @@
-<!--
-    REMINDER for core developers only:
-    Remember to raise Dask migration work PRs against the lama-to-dask branch
-    rather than master!
-  -->
diff --git a/Changelog.rst b/Changelog.rst
index 9412b14b2..f61fec275 100644
--- a/Changelog.rst
+++ b/Changelog.rst
@@ -1,16 +1,3 @@
-version 3.13.0
---------------
-
-**2022-0?-??**
-
-* Fix bug in `cf.read` when reading PP or fields files for which the
-  ``um`` keyword has been set, but without the ``'version'`` key
-  (https://github.com/NCAS-CMS/cf-python/issues/306)
-* Changed dependency: ``1.9.0.2<=cfdm<1.9.1.0``
-* New dependency: ``packaging>=20.0``
-
-----
-
 version 3.12.0
 --------------
 
diff --git a/cf/__init__.py b/cf/__init__.py
index fb2c9232b..7b889d73f 100644
--- a/cf/__init__.py
+++ b/cf/__init__.py
@@ -96,13 +96,13 @@ except ImportError as error1:
 
 __cf_version__ = cfdm.core.__cf_version__
 
-from packaging.version import Version
+from distutils.version import LooseVersion
 import importlib.util
 import platform
 
 # Check the version of Python
 _minimum_vn = "3.7.0"
-if Version(platform.python_version()) < Version(_minimum_vn):
+if LooseVersion(platform.python_version()) < LooseVersion(_minimum_vn):
     raise ValueError(
         f"Bad python version: cf requires python version {_minimum_vn} "
         f"or later. Got {platform.python_version()}"
@@ -141,7 +141,7 @@ except ImportError as error1:
 
 # Check the version of psutil
 _minimum_vn = "0.6.0"
-if Version(psutil.__version__) < Version(_minimum_vn):
+if LooseVersion(psutil.__version__) < LooseVersion(_minimum_vn):
     raise RuntimeError(
         f"Bad psutil version: cf requires psutil>={_minimum_vn}. "
         f"Got {psutil.__version__} at {psutil.__file__}"
@@ -149,7 +149,7 @@ if Version(psutil.__version__) < Version(_minimum_vn):
 
 # Check the version of netCDF4
 _minimum_vn = "1.5.4"
-if Version(netCDF4.__version__) < Version(_minimum_vn):
+if LooseVersion(netCDF4.__version__) < LooseVersion(_minimum_vn):
     raise RuntimeError(
         f"Bad netCDF4 version: cf requires netCDF4>={_minimum_vn}. "
         f"Got {netCDF4.__version__} at {netCDF4.__file__}"
@@ -157,7 +157,7 @@ if Version(netCDF4.__version__) < Version(_minimum_vn):
 
 # Check the version of cftime
 _minimum_vn = "1.5.0"
-if Version(cftime.__version__) < Version(_minimum_vn):
+if LooseVersion(cftime.__version__) < LooseVersion(_minimum_vn):
     raise RuntimeError(
         f"Bad cftime version: cf requires cftime>={_minimum_vn}. "
         f"Got {cftime.__version__} at {cftime.__file__}"
@@ -165,7 +165,7 @@ if Version(cftime.__version__) < Version(_minimum_vn):
 
 # Check the version of numpy
 _minimum_vn = "1.15"
-if Version(numpy.__version__) < Version(_minimum_vn):
+if LooseVersion(numpy.__version__) < LooseVersion(_minimum_vn):
     raise RuntimeError(
         f"Bad numpy version: cf requires numpy>={_minimum_vn}. "
         f"Got {numpy.__version__} at {numpy.__file__}"
@@ -173,17 +173,17 @@ if Version(numpy.__version__) < Version(_minimum_vn):
 
 # Check the version of cfunits
 _minimum_vn = "3.3.4"
-if Version(cfunits.__version__) < Version(_minimum_vn):
+if LooseVersion(cfunits.__version__) < LooseVersion(_minimum_vn):
     raise RuntimeError(
         f"Bad cfunits version: cf requires cfunits>={_minimum_vn}. "
         f"Got {cfunits.__version__} at {cfunits.__file__}"
     )
 
 # Check the version of cfdm
-_minimum_vn = "1.9.0.2"
+_minimum_vn = "1.9.0.1"
 _maximum_vn = "1.9.1.0"
-_cfdm_version = Version(cfdm.__version__)
-if not Version(_minimum_vn) <= _cfdm_version < Version(_maximum_vn):
+_cfdm_version = LooseVersion(cfdm.__version__)
+if not LooseVersion(_minimum_vn) <= _cfdm_version < LooseVersion(_maximum_vn):
     raise RuntimeError(
         f"Bad cfdm version: cf requires {_minimum_vn}<=cfdm<{_maximum_vn}. "
         f"Got {_cfdm_version} at {cfdm.__file__}"
diff --git a/cf/aggregate.py b/cf/aggregate.py
index 39d3a2c10..cf218fc7a 100644
--- a/cf/aggregate.py
+++ b/cf/aggregate.py
@@ -214,7 +214,7 @@ class _Meta:
         self.has_data = f.has_data()
         self.identity = f.identity(
             strict=strict_identities,
-            relaxed=relaxed_identities,
+            relaxed=relaxed_identities and not ncvar_identities,
             nc_only=ncvar_identities,
             default=None,
         )
@@ -309,7 +309,7 @@ class _Meta:
         else:
             self.coordrefs = list(refs.values())
 
-        for axis in f.domain_axes(todict=True):
+        for axis, domain_axis in f.domain_axes(todict=True).items():
 
             # List some information about each 1-d coordinate which
             # spans this axis. The order of elements is arbitrary, as
@@ -400,27 +400,21 @@ class _Meta:
             # auxiliary coordinate information
             info_1d_coord = info_dim + info_aux
 
-            #            if not info_1d_coord:
-            #                self.message = ("axis has no one-dimensional nor "
-            #                                    "scalar coordinates")
-            #                return
-
             # Find the canonical identity for this axis
             identity = None
             if info_1d_coord:
                 identity = info_1d_coord[0]["identity"]
             elif not self.relaxed_identities:
                 self.message = (
-                    "axis has no one-dimensional nor " "scalar coordinates"
+                    "axis has no one-dimensional nor scalar coordinates"
                 )
                 return
 
-            ncdim = False
+            size = None
             if identity is None and self.relaxed_identities:
-                # There are no 1-d coordinates, so see if we can
-                # identify the domain axis by their netCDF dimension
-                # name.
-                domain_axis = f.axis(axis)  # TODO
+                # There are no 1-d coordinates and relaxed identities
+                # are on, so see if we can identify the domain axis by
+                # its netCDF dimension name.
                 identity = domain_axis.nc_get_dimension(None)
                 if identity is None:
                     self.message = (
@@ -428,8 +422,8 @@ class _Meta:
                         "has no netCDF dimension name"
                     )  # TODO
                     return
-                else:
-                    ncdim = True
+
+                size = domain_axis.get_size()
 
             axis_identities = {
                 "ids": "identity",
@@ -438,7 +432,6 @@ class _Meta:
                 "hasdata": "hasdata",
                 "hasbounds": "hasbounds",
                 "coordrefs": "coordrefs",
-                # 'size': None,
             }
             self.axis[identity] = {
                 name: tuple(i[idt] for i in info_1d_coord)
@@ -450,11 +443,10 @@ class _Meta:
             else:
                 self.axis[identity]["dim_coord_index"] = None
 
-            # Store the axis size if the axis has no 1-d coordinates
-            if ncdim:
-                self.axis[identity]["size"] = domain_axis.get_size()
-            else:
-                self.axis[identity]["size"] = None
+            # Store the axis size, which will be None unless we
+            # identified the dimension solely by its netCDF dimension
+            # name.
+            self.axis[identity]["size"] = size
 
             self.id_to_axis[identity] = axis
             self.axis_to_id[axis] = identity
@@ -933,7 +925,7 @@ class _Meta:
         """
         identity = coord.identity(
             strict=self.strict_identities,
-            relaxed=self.relaxed_identities,
+            relaxed=self.relaxed_identities and not self.ncvar_identities,
             nc_only=self.ncvar_identities,
             default=None,
         )
@@ -973,7 +965,7 @@ class _Meta:
         """
         identity = anc.identity(
             strict=self.strict_identities,
-            relaxed=self.relaxed_identities,
+            relaxed=self.relaxed_identities and not self.ncvar_identities,
             nc_only=self.ncvar_identities,
             default=None,
         )
@@ -1055,7 +1047,7 @@ class _Meta:
         else:
             anc_identity = anc.identity(
                 strict=self.strict_identities,
-                relaxed=self.relaxed_identities,
+                relaxed=self.relaxed_identities and not self.ncvar_identities,
                 nc_only=self.ncvar_identities,
                 default=None,
             )
@@ -1459,7 +1451,9 @@ def aggregate(
         relaxed_identities: `bool`, optional
             If True and there is no standard name property nor "id"
             attribute, then allow field and metadata constructs to be
-            identifiable by long name properties or netCDF variable names.
+            identifiable by long name properties or netCDF variable
+            names. Also allows netCDF dimension names to be used when
+            there are no spanning 1-d coordinates.
 
         field_identity: `str`, optional
             Specify a property with which to identify field constructs
@@ -1877,7 +1871,7 @@ def aggregate(
 
                 coord_identity = coord.identity(
                     strict=strict_identities,
-                    relaxed=relaxed_identities,
+                    relaxed=relaxed_identities and not ncvar_identities,
                     nc_only=ncvar_identities,
                     default=None,
                 )
diff --git a/cf/read_write/read.py b/cf/read_write/read.py
index 55dc374d3..edb47913d 100644
--- a/cf/read_write/read.py
+++ b/cf/read_write/read.py
@@ -434,44 +434,44 @@ def read(
             ``'fmt'``                     The file format (``'PP'`` or
                                           ``'FF'``)
 
-            ``'word_size'``               The word size in bytes
-                                          (``4`` or ``8``).
+            ``'word_size'``               The word size in bytes (``4`` or
+                                          ``8``)
 
             ``'endian'``                  The byte order (``'big'`` or
-                                          ``'little'``).
+                                          ``'little'``)
 
             ``'version'``                 The UM version to be used
                                           when decoding the
-                                          header. Valid versions are,
-                                          for example, ``4.2``,
+                                          header. Valid versions
+                                          are, for example, ``4.2``,
                                           ``'6.6.3'`` and
-                                          ``'8.2'``. In general, a
-                                          given version is ignored if
-                                          it can be inferred from the
-                                          header (which is usually the
-                                          case for files created by
-                                          the UM at versions 5.3 and
+                                          ``'8.2'``. The default
+                                          version is ``4.5``. In
+                                          general, a given version
+                                          is ignored if it can be
+                                          inferred from the header
+                                          (which is usually the case
+                                          for files created by the
+                                          UM at versions 5.3 and
                                           later). The exception to
                                           this is when the given
-                                          version has a third element
-                                          (such as the 3 in 6.6.3), in
-                                          which case any version in
-                                          the header is ignored.
-
-                                          The default version is
-                                          ``4.5``.
+                                          version has a third
+                                          element (such as the 3 in
+                                          6.6.3), in which case any
+                                          version in the header is
+                                          ignored.
 
             ``'height_at_top_of_model'``  The height (in metres) of
                                           the upper bound of the top
-                                          model level. By default the
-                                          height at top model is taken
-                                          from the top level's upper
-                                          bound defined by BRSVD1 in
-                                          the lookup header. If the
-                                          height at top model can not
-                                          be determined from the
-                                          header and is not provided
-                                          then no
+                                          model level. By default
+                                          the height at top model is
+                                          taken from the top level's
+                                          upper bound defined by
+                                          BRSVD1 in the lookup
+                                          header. If the height at
+                                          top model can not be
+                                          determined from the header and is
+                                          not provided then no
                                           "atmosphere_hybrid_height_coordinate"
                                           dimension coordinate
                                           construct will be created.
@@ -723,9 +723,25 @@ def read(
                     ftype = file_type(filename)
                 except Exception as error:
                     if not ignore_read_error:
-                        raise ValueError(error)
+                        message = error
+
+                        #                        if not find_library("umfile"):
+                        #                            message += ("\n\n"
+                        #                                "Note: Unable to detect the UM read C library needed "
+                        #                                "to recognise and read PP and UM fields files. "
+                        #                                "This indicates a compilation problem during the "
+                        #                                "cf installation (though note it does not affect "
+                        #                                "any other cf functionality, notably netCDF file "
+                        #                                "processing). If processing of PP and FF files is "
+                        #                                "required, ensure 'GNU make' is available and "
+                        #                                "reinstall cf-python to try to build the library. "
+                        #                                "Note a warning will be given if the build fails."
+                        #                            )
+
+                        raise ValueError(message)
 
                     logger.warning(f"WARNING: {error}")  # pragma: no cover
+
                     continue
 
             if domain and ftype == "UM":
@@ -914,17 +930,34 @@ def _read_a_file(
     umversion = 405
 
     if um:
+        # ftype = 'UM'
         fmt = um.get("fmt")
         word_size = um.get("word_size")
         endian = um.get("endian")
-        umversion = um.get("version", umversion)
+        umversion = um.get("version")
         height_at_top_of_model = um.get("height_at_top_of_model")
-
-        if fmt is not None:
+        if fmt in ("PP", "pp", "pP", "Pp"):
             fmt = fmt.upper()
+            # For PP format, there is a default word size and
+            # endian-ness
+            if word_size is None:
+                word_size = 4
+
+            if endian is None:
+                endian = "big"
 
         if umversion is not None:
             umversion = float(str(umversion).replace(".", "0", 1))
+    #    else:
+    #        try:
+    #            ftype = file_type(filename)
+    #        except Exception as error:
+    #            if not ignore_read_error:
+    #                raise Exception(error)
+    #
+    #            logger.warning('WARNING: {}'.format(error))  # pragma: no cover
+    #
+    #            return FieldList()
 
     extra_read_vars = {
         "chunk": chunk,
diff --git a/cf/umread_lib/c-lib/Makefile b/cf/umread_lib/c-lib/Makefile
index 811117797..59f5b8d23 100644
--- a/cf/umread_lib/c-lib/Makefile
+++ b/cf/umread_lib/c-lib/Makefile
@@ -1,7 +1,7 @@
 HEADERS=umfile.h umfileint.h bits/*.h
 
 LIBRARY=umfile.so
-CFLAGS=-Wall -fPIC
+CFLAGS=-Wall -fPIC -g
 
 UNAME_S := $(shell uname -s)
 ifeq ($(UNAME_S),Linux)
diff --git a/docs/source/installation.rst b/docs/source/installation.rst
index 3c719b20f..8c30d79ea 100644
--- a/docs/source/installation.rst
+++ b/docs/source/installation.rst
@@ -190,15 +190,13 @@ Required
 * `cftime <https://pypi.org/project/cftime/>`_, version 1.5.0 or newer
   (note that this package may be installed with netCDF4).
 
-* `cfdm <https://pypi.org/project/cfdm/>`_, version 1.9.0.2 or up to,
+* `cfdm <https://pypi.org/project/cfdm/>`_, version 1.9.0.1 or up to,
   but not including, 1.9.1.0.
 
 * `cfunits <https://pypi.org/project/cfunits/>`_, version 3.3.4 or newer.
 
 * `psutil <https://pypi.org/project/psutil/>`_, version 0.6.0 or newer.
 
-* `packaging <https://pypi.org/project/packaging/>`_, version 20.0 or newer.
-
 .. _UNIDATA-UDUNITS-2-library:
 
 * `UNIDATA UDUNITS-2 library
diff --git a/docs/source/tutorial.rst b/docs/source/tutorial.rst
index fc68ec0eb..a2391a010 100644
--- a/docs/source/tutorial.rst
+++ b/docs/source/tutorial.rst
@@ -2334,7 +2334,7 @@ Date-time ranges that span the time duration can also be created:
 Time duration constructors
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-For convenience, `cf.TimeDuration` instances can be created with the
+For convenience, `cf.TtimeDuration` instances can be created with the
 following constructors:
 
 ===========  ===========================================================
diff --git a/requirements.txt b/requirements.txt
index d8ec98819..69def82fe 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,7 +1,6 @@
 netCDF4>=1.5.4
 cftime>=1.5.0
 numpy>=1.15
-cfdm>=1.9.0.2, <1.9.1.0
+cfdm>=1.9.0.1, <1.9.1.0
 psutil>=0.6.0
 cfunits>=3.3.4
-packaging>=20.0
diff --git a/scripts/cfa b/scripts/cfa
index 26a7c8cde..8bf084cd3 100755
--- a/scripts/cfa
+++ b/scripts/cfa
@@ -5,7 +5,6 @@ if __name__ == "__main__":
 
     import os
     import sys
-    import traceback
     from getopt import GetoptError, getopt
     from re import sub as re_sub
 
@@ -1488,10 +1487,9 @@ Using cf-python library version {cf.__version__} at {library_path}"""
         # ------------------------------------------------------------
         try:
             f = cf.read(infile, **read_options)
-        except Exception:
-            traceback.print_exc()
+        except Exception as error:
             print(
-                f"\n{iam} ERROR reading {infile}",
+                f"{iam} ERROR reading {infile}: {error}",
                 file=sys.stderr,
             )
             sys.exit(1)
@@ -1517,10 +1515,9 @@ Using cf-python library version {cf.__version__} at {library_path}"""
 
             try:
                 cf.write(f, outfile, **write_options)
-            except Exception:
-                traceback.print_exc()
+            except Exception as error:
                 print(
-                    f"\n{iam} ERROR writing {outfile}",
+                    f"{iam} ERROR writing {outfile}: {error}",
                     file=sys.stderr,
                 )
                 sys.exit(1)


Test patch:
diff --git a/cf/test/test_aggregate.py b/cf/test/test_aggregate.py
index 2679eb97a..c90a7bc5c 100644
--- a/cf/test/test_aggregate.py
+++ b/cf/test/test_aggregate.py
@@ -271,6 +271,14 @@ class aggregateTest(unittest.TestCase):
         a = a[0]
         self.assertEqual(len(a.auxiliary_coordinates()), 1)
 
+    def test_aggregate_keyword_consistency(self):
+        """Test acceptable keyword combinations."""
+        f = cf.example_field(0)
+        a = cf.aggregate(
+            [f[:2], f[2:]], relaxed_identities=True, ncvar_identities=True
+        )
+        self.assertEqual(len(a), 1)
+
 
 if __name__ == "__main__":
     print("Run date:", datetime.datetime.now())
diff --git a/cf/test/test_pp.py b/cf/test/test_pp.py
index 91180071e..9255f5c04 100644
--- a/cf/test/test_pp.py
+++ b/cf/test/test_pp.py
@@ -124,16 +124,6 @@ class ppTest(unittest.TestCase):
         self.assertTrue(f.auxiliary_coordinate("longitude", default=False))
         self.assertTrue(f.auxiliary_coordinate("longitude", default=False))
 
-    def test_PP_read_um(self):
-        f = cf.read(self.ppextradata)[0]
-
-        g = cf.read(self.ppextradata, um={"fmt": "pp"})[0]
-        self.assertTrue(f.equals(g))
-
-        for vn in (4.5, 405, "4.5", None):
-            g = cf.read(self.ppextradata, um={"fmt": "pp", "version": vn})[0]
-            self.assertTrue(f.equals(g))
-
 
 if __name__ == "__main__":
     print("Run date:", datetime.datetime.now())