Repository: ioos/compliance-checker (python)

Issue:
CF Checker Plugin: extend notification if check for order of dimension fails
# Current situation

Dimensions of data variables are recommended to be ordered as follows:

* spatio-temporal dimensions (e.g. X, Y, Z and T) should be in the **order**: **TZYX**
* **additional dimensions** should be located to the **left** of the spatio-temporal dimensions

This is recommended in [section 2.4](http://cfconventions.org/Data/cf-conventions/cf-conventions-1.8/cf-conventions.html#dimensions) of the CF Conventions:

> If any or all of the dimensions of a variable have the interpretations of "date or time" (T), "height or depth" (Z), "latitude" (Y), or "longitude" (X) then we recommend, but do not require (see Section 1.4, "Relationship to the COARDS Conventions" ), those dimensions to appear in the relative order T, then Z, then Y, then X in the CDL definition corresponding to the file. All other dimensions should, whenever possible, be placed to the left of the spatiotemporal dimensions.

The check for this is done in `cf.CFBaseCheck._dims_in_order` and looks as follows:

```python
    def _dims_in_order(self, dimension_order):
        """
        :param list dimension_order: A list of axes
        :rtype: bool
        :return: Returns True if the dimensions are in order U*, T, Z, Y, X,
                 False otherwise
        """

        regx = regex.compile(r"^[^TZYX]*T?Z?Y?X?$")
        dimension_string = "".join(dimension_order)
        return regx.match(dimension_string) is not None
```

# Issue

If the user gets

```
VARIBALE's dimensions are not in the recommended order T, Z, Y, X. They are DIMENSIONs
```

it it not obvious whether

1. one or more of the dimensions T, Z, Y and/or X were not properly recognized, 
2. an additional dimension is _wrongly_ located, or
3. the dimensions T, Z, Y and/or X are actually wrongly ordered.


# Solution

Extend the user output. Although quite long, I would suggest the following text: 

```
VARIBALE's spatio-temporal dimensions are not in the recommended order T, Z, Y, X and/or further dimensions are not located left of T, Z, Y, X.  The dims are DIMENSIONs. Their guessed types are DIMENSION_TYPES (U: other/unknown; L: unlimited).
```

When there is agreement reached, I will be happy to create a PR.


# Outlook / Addtional Question

Considering unlimited dimensions as L-type and not as T-, Z-, Y- or X-type (if they were of such a type) might cause issues. E.g. if the Y dimension was unlimited, we would get a warning in the current situation.

[A reply](https://github.com/cf-convention/discuss/issues/60#issuecomment-661827686) to my [issue on order of dimensions in timeSeries datasets in the CF Discussion Repo](https://github.com/cf-convention/discuss/issues/60) suggest that the requirement for unlimited dimensions to be located on the left ([CF Section 9.3](http://cfconventions.org/Data/cf-conventions/cf-conventions-1.8/cf-conventions.html#representations-features)), might be related to the technical limitation of netCDF3 (one unlimited dim; unlimited dim most left):

> If there is a need for either the instance or an element dimension to be the netCDF unlimited dimension (so that more features or more elements can be appended), then that dimension must be the outer dimension of the data variable i.e. the leading dimension in CDL.

This means that the identification of unlimited dimensions in the IOOS CC CF Plugin could drop. Similarly the dimension type `L` could be dropped, which would simplify checking the order of dimensions.

Pull request:
improve warning message when dimension order is bad (CF 2.4)
fixed #836 

* update message from `VARIBALE's dimensions are not in the recommended order T, Z, Y, X. They are DIMENSIONs` to `VARIBALE's spatio-temporal dimensions are not in the recommended order T, Z, Y, X and/or further dimensions are not located left of T, Z, Y, X.  The dims are DIMENSIONs. Their guessed types are DIMENSION_TYPES (U: other/unknown; L: unlimited).`.
* updated unit test, which checks this warning message

Patch:
diff --git a/compliance_checker/cf/cf.py b/compliance_checker/cf/cf.py
index 1c7d3dd..dde92e7 100644
--- a/compliance_checker/cf/cf.py
+++ b/compliance_checker/cf/cf.py
@@ -950,6 +950,29 @@ class CFBaseCheck(BaseCheck):
             dim_names.append(dim_name)
         return ", ".join(dim_names)
 
+    def _get_pretty_dimension_order_with_type(self, ds, name, dim_types):
+        """
+        Returns a comma separated string of the dimensions for a specified
+        variable of format "DIMENSIONS_NAME (DIMENSION_TYPE[, unlimited])"
+
+        :param netCDF4.Dataset ds: An open netCDF dataset
+        :param str name: A string with a valid NetCDF variable name for the
+                         dataset
+        :param list dim_types: A list of strings returned by
+                               _get_dimension_order for the same "name"
+        :rtype: str
+        :return: A comma separated string of the variable's dimensions
+        """
+        dim_names = []
+        for dim, dim_type in zip(ds.variables[name].dimensions, dim_types):
+            dim_name = "{} ({}".format(dim, dim_type)
+            if ds.dimensions[dim].isunlimited():
+                dim_name += ", unlimited)"
+            else:
+                dim_name += ")"
+            dim_names.append(dim_name)
+        return ", ".join(dim_names)
+
     def _is_station_var(self, var):
         """
         Returns True if the NetCDF variable is associated with a station, False
@@ -1554,9 +1577,16 @@ class CF1_6Check(CFNCCheck):
                 dimension_order = self._get_dimension_order(ds, name, coord_axis_map)
                 valid_dimension_order.assert_true(
                     self._dims_in_order(dimension_order),
-                    "{}'s dimensions are not in the recommended order "
-                    "T, Z, Y, X. They are {}"
-                    "".format(name, self._get_pretty_dimension_order(ds, name)),
+                    "{}'s spatio-temporal dimensions are not in the "
+                    "recommended order T, Z, Y, X and/or further dimensions "
+                    "are not located left of T, Z, Y, X. The dimensions (and "
+                    "their guessed types) are {} (with U: other/unknown; L: "
+                    "unlimited).".format(
+                        name,
+                        self._get_pretty_dimension_order_with_type(
+                            ds, name, dimension_order
+                        ),
+                    ),
                 )
         return valid_dimension_order.to_result()
 


Test patch:
diff --git a/compliance_checker/tests/test_cf.py b/compliance_checker/tests/test_cf.py
index 86caef0..03b9284 100644
--- a/compliance_checker/tests/test_cf.py
+++ b/compliance_checker/tests/test_cf.py
@@ -307,8 +307,11 @@ class TestCF1_6(BaseTestCase):
         result = self.cf.check_dimension_order(dataset)
         assert result.value == (5, 6)
         assert result.msgs[0] == (
-            u"really_bad's dimensions are not in the recommended order "
-            "T, Z, Y, X. They are latitude, power"
+            u"really_bad's spatio-temporal dimensions are not in the "
+            "recommended order T, Z, Y, X and/or further dimensions are not "
+            "located left of T, Z, Y, X. The dimensions (and their guessed "
+            "types) are latitude (Y), power (U) (with U: other/unknown; L: "
+            "unlimited)."
         )
 
         dataset = self.load_dataset(STATIC_FILES["dimension_order"])