Repository: ioos/compliance-checker
Pull request #913 (2022-03-24 21:31:02)

Fix integration tests rebase
Fixes issues in integration tests.

Other issues addressed
- Remove bounds from consideration of geophysical variables
- Add in associated standard names for computed standard names used in formula_terms.


Patch:
diff --git a/compliance_checker/cf/appendix_d.py b/compliance_checker/cf/appendix_d.py
index b71dab9..70c13e5 100644
--- a/compliance_checker/cf/appendix_d.py
+++ b/compliance_checker/cf/appendix_d.py
@@ -27,11 +27,23 @@ is omitted from the formula_terms attribute should be assumed to be zero.
 # vertical coordinates only have one computed_standard_name, but some have
 # multiple acceptable values.
 ocean_computed_standard_names = {
-    "altitude",
-    "height_above_geopotential_datum",
-    "height_above_reference_ellipsoid",
-    "height_above_mean_sea_level",
+    "altitude": {"zlev": "altitude",
+                 "eta": "sea_surface_height_above_geoid",
+                 "depth": "sea_floor_depth_below_geoid"},
+    "height_above_geopotential_datum":
+       {"zlev": "height_above_geopotential_datum",
+        "eta": "sea_surface_height_above_geopotential_datum",
+        "depth": "sea_floor_depth_below_geopotential_datum"},
+    "height_above_reference_ellipsoid":
+        {"zlev": "height_above_reference_ellipsoid",
+         "eta": "sea_surface_height_above_reference_ellipsoid",
+         "depth": "sea_floor_depth_below_reference_ellipsoid"},
+    "height_above_mean_sea_level":
+         {"zlev": "height_above_mean_sea_level",
+          "eta": "sea_surface_height_above_mean_sea_level",
+          "depth": "sea_floor_depth_below_mean_sea_level"}
 }
+
 dimless_vertical_coordinates_1_6 = {  # only for CF-1.6
     "atmosphere_ln_pressure_coordinate": ({"p0", "lev"}, {"air_pressure"}),
     "atmosphere_sigma_coordinate": ({"sigma", "ps", "ptop"}, {"air_pressure"}),
diff --git a/compliance_checker/cf/cf_1_6.py b/compliance_checker/cf/cf_1_6.py
index 586d9df..cf80ee1 100644
--- a/compliance_checker/cf/cf_1_6.py
+++ b/compliance_checker/cf/cf_1_6.py
@@ -38,6 +38,7 @@ class CF1_6Check(CFNCCheck):
     _cc_url = "http://cfconventions.org/cf-conventions/v1.6.0/cf-conventions.html"
     _cc_display_headers = {3: "Errors", 2: "Warnings", 1: "Info"}
     appendix_a = appendix_a_base
+    appendix_d_parametric_coords = dimless_vertical_coordinates_1_6
 
     def __init__(self, options=None):  # initialize with parent methods and data
         super(CF1_6Check, self).__init__(options)
@@ -565,13 +566,18 @@ class CF1_6Check(CFNCCheck):
         modifier_variables = cfutil._find_standard_name_modifier_variables(ds)
         forecast_variables = cfutil.get_forecast_metadata_variables(ds)
 
-        unit_required_variables = set(
+        dimless_vert = {var.name for var in
+                        ds.get_variables_by_attributes(standard_name=lambda s: s in self.appendix_d_parametric_coords)
+                        if not hasattr(var, "units")}
+        # check anything remaining that has units
+        #unit_containing =
+        unit_required_variables = (set(
             coordinate_variables
             + auxiliary_coordinates
             + geophysical_variables
             + forecast_variables
-            + modifier_variables # standard names with modifiers require proper units, *except* for flags, where they should not be present
-        )
+            + modifier_variables) # standard names with modifiers require proper units, *except* for flags, where they should not be present
+            - dimless_vert)
 
         for name in unit_required_variables:
             # For reduced horizontal grids, the compression index variable does
@@ -605,8 +611,8 @@ class CF1_6Check(CFNCCheck):
                                            self.section_titles["3.1"])
 
             # side effects, but better than teasing out the individual result
-            if units_attr_is_string.assert_true(
-                isinstance(units, str) or units is None,
+            if units is not None and units_attr_is_string.assert_true(
+                isinstance(units, str),
                 "units ({}) attribute of '{}' must be a string compatible with UDUNITS".format(
                     units, variable.name
                 ),
@@ -655,10 +661,7 @@ class CF1_6Check(CFNCCheck):
         #    status_flag
         if standard_name_modifier is not None:
             if standard_name_modifier not in valid_modifiers:
-                valid_units.out_of += 1
-                message = (f"Standard name modifier {standard_name_modifier} is not one of "
-                           f"{valid_modifiers.keys()}")
-                valid_units.messages.append(message)
+                # standard name modifier warning given elsewhere
                 return valid_units.to_result()
             else:
                 unit_type = valid_modifiers[standard_name_modifier]
@@ -718,11 +721,14 @@ class CF1_6Check(CFNCCheck):
             valid_units.score += 1
             valid_units.out_of += 1
 
-        # time has special unit handling rules
-        if standard_name != "time":
+        # time and forecast_reference time have special unit handling rules
+        # that use time relative to a reference point, despite canonical units
+        # being expressed as "s"/seconds
+        if standard_name not in {"time", "forecast_reference_time"}:
             valid_units.assert_true(units_conv.is_convertible(Unit(reference)),
-                                    f'Units "{units}" must be convertible to canonical units '
-                                    f'"{reference}"')
+                                    f'Units "{units}" for variable '
+                                    f"{variable_name} must be convertible to "
+                                    f'canonical units "{reference}"')
 
         return valid_units.to_result()
 
@@ -823,7 +829,7 @@ class CF1_6Check(CFNCCheck):
             # degrees is allowed if using a transformed grid
             allowed_units = cfutil.VALID_LAT_UNITS | {"degrees"}
             valid_standard_units.assert_true(
-                units.lower() if units is not None else None in allowed_units,
+                (units.lower() if units is not None else None) in allowed_units,
                 'variables defining latitude ("{}") must use degrees_north '
                 "or degrees if defining a transformed grid. Currently "
                 "{}".format(variable_name, units),
@@ -833,7 +839,7 @@ class CF1_6Check(CFNCCheck):
             # degrees is allowed if using a transformed grid
             allowed_units = cfutil.VALID_LON_UNITS | {"degrees"}
             valid_standard_units.assert_true(
-                units.lower() if units is not None else None in allowed_units,
+                (units.lower() if units is not None else None) in allowed_units,
                 'variables defining longitude ("{}") must use degrees_east '
                 "or degrees if defining a transformed grid. Currently "
                 "{}".format(variable_name, units),
diff --git a/compliance_checker/cfutil.py b/compliance_checker/cfutil.py
index 3d06c85..1019170 100644
--- a/compliance_checker/cfutil.py
+++ b/compliance_checker/cfutil.py
@@ -374,6 +374,13 @@ def get_cell_boundary_variables(ds):
             boundary_variables.append(var.bounds)
     return boundary_variables
 
+@lru_cache(128)
+def get_bounds_variables(ds):
+    contains_bounds = ds.get_variables_by_attributes(bounds=
+                                                     lambda s: s in ds.variables)
+    return {ds.variables[parent_var.bounds] for parent_var in contains_bounds}
+
+
 
 @lru_cache(128)
 def get_geophysical_variables(ds):
@@ -383,10 +390,10 @@ def get_geophysical_variables(ds):
 
     :param netCDF4.Dataset nc: An open netCDF dataset
     """
-
     parameters = []
     for variable in ds.variables:
-        if is_geophysical(ds, variable):
+        if (is_geophysical(ds, variable) and
+            variable not in get_bounds_variables(ds)):
             parameters.append(variable)
     return parameters
 


Test patch:
diff --git a/compliance_checker/tests/test_cf_integration.py b/compliance_checker/tests/test_cf_integration.py
index eddc94f..e0b1445 100644
--- a/compliance_checker/tests/test_cf_integration.py
+++ b/compliance_checker/tests/test_cf_integration.py
@@ -39,29 +39,37 @@ dataset_stem__expected_messages = [
     (
         "l01-met",
         [
-            "units for variable air_temperature_qc must be convertible to K currently they are 1",
-            "units for variable wind_speed_qc must be convertible to m s-1 currently they are 1",
+            "Attribute 'valid_range' (type: <class 'numpy.int16'>) and parent variable 'air_temperature_qc' (type: <class 'numpy.int8'>) must have equivalent datatypes",
+            "Attribute 'valid_range' (type: <class 'numpy.int16'>) and parent variable 'barometric_pressure_qc' (type: <class 'numpy.int8'>) must have equivalent datatypes",
+            "Attribute 'valid_range' (type: <class 'numpy.float64'>) and parent variable 'wind_gust' (type: <class 'numpy.float32'>) must have equivalent datatypes",
+            "Attribute 'valid_range' (type: <class 'numpy.int16'>) and parent variable 'wind_gust_qc' (type: <class 'numpy.int8'>) must have equivalent datatypes",
+            "Attribute 'valid_range' (type: <class 'numpy.float64'>) and parent variable 'wind_speed' (type: <class 'numpy.float32'>) must have equivalent datatypes",
+            "Attribute 'valid_range' (type: <class 'numpy.int16'>) and parent variable 'wind_speed_qc' (type: <class 'numpy.int8'>) must have equivalent datatypes",
+            "Attribute 'valid_range' (type: <class 'numpy.float64'>) and parent variable 'wind_direction' (type: <class 'numpy.float32'>) must have equivalent datatypes",
+            "Attribute 'valid_range' (type: <class 'numpy.int16'>) and parent variable 'wind_direction_qc' (type: <class 'numpy.int8'>) must have equivalent datatypes",
+            "Attribute 'valid_range' (type: <class 'numpy.int16'>) and parent variable 'visibility_qc' (type: <class 'numpy.int8'>) must have equivalent datatypes",
+            '§2.6.1 Conventions global attribute does not contain "CF-1.8"',
             "standard_name visibility is not defined in Standard Name Table v{}".format(
                 std_names._version
             ),
-            "standard_name modifier data_quality for variable visibility_qc is not a valid modifier according to appendix C",
+            "Standard name modifier \"data_quality\" for variable visibility_qc is not a valid modifier according to CF Appendix C",
             "standard_name wind_direction is not defined in Standard Name Table v{}".format(
                 std_names._version
             ),
-            "standard_name modifier data_quality for variable wind_direction_qc is not a valid modifier according to appendix C",
+            "Standard name modifier \"data_quality\" for variable wind_direction_qc is not a valid modifier according to CF Appendix C",
             "standard_name wind_gust is not defined in Standard Name Table v{}".format(
                 std_names._version
             ),
-            "standard_name modifier data_quality for variable wind_gust_qc is not a valid modifier according to appendix C",
-            "standard_name modifier data_quality for variable air_temperature_qc is not a valid modifier according to appendix C",
+            "Standard name modifier \"data_quality\" for variable wind_gust_qc is not a valid modifier according to CF Appendix C",
+            "Standard name modifier \"data_quality\" for variable air_temperature_qc is not a valid modifier according to CF Appendix C",
             "standard_name use_wind is not defined in Standard Name Table v{}".format(
                 std_names._version
             ),
             "standard_name barometric_pressure is not defined in Standard Name Table v{}".format(
                 std_names._version
             ),
-            "standard_name modifier data_quality for variable barometric_pressure_qc is not a valid modifier according to appendix C",
-            "standard_name modifier data_quality for variable wind_speed_qc is not a valid modifier according to appendix C",
+            "Standard name modifier \"data_quality\" for variable barometric_pressure_qc is not a valid modifier according to CF Appendix C",
+            "Standard name modifier \"data_quality\" for variable wind_speed_qc is not a valid modifier according to CF Appendix C",
             "standard_name barometric_pressure is not defined in Standard Name Table v{}".format(
                 std_names._version
             ),
@@ -87,7 +95,6 @@ dataset_stem__expected_messages = [
         "ooi_glider",
         [
             u"§2.6.2 comment global attribute should be a non-empty string",
-            u"units (None) attribute of 'deployment' must be a string compatible with UDUNITS",
             u"Attribute long_name or/and standard_name is highly recommended for variable deployment",
             u"latitude variable 'latitude' should define standard_name='latitude' or axis='Y'",
             u"longitude variable 'longitude' should define standard_name='longitude' or axis='X'",
@@ -98,8 +105,7 @@ dataset_stem__expected_messages = [
         [
             "global attribute _CoordSysBuilder should begin with a letter and be composed of letters, digits, and underscores",
             '§2.6.1 Conventions global attribute does not contain "CF-1.8"',
-            "units for variable time_offset must be convertible to s currently they are hours since 2013-02-18T00:00:00Z",
-            "units for variable time_run must be convertible to s currently they are hours since 2013-02-18 00:00:00.000 UTC",
+            'Units "hours since 2013-02-18T00:00:00Z" for variable time_offset must be convertible to canonical units "s"',
             "lon's axis attribute must be T, X, Y, or Z, currently x",
             "lat's axis attribute must be T, X, Y, or Z, currently y",
             "z's axis attribute must be T, X, Y, or Z, currently z",
@@ -114,6 +120,7 @@ dataset_stem__expected_messages = [
     (
         "pr_inundation",
         [
+            'Units "degrees2" for variable area must be convertible to canonical units "m2"',
             "waterlevel'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 time (T), m (A), n (A) (with U: other/unknown; L: unlimited).",
             "velocity_x'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 time (T), Layer (Z), m (A), n (A) (with U: other/unknown; L: unlimited).",
             "velocity_y'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 time (T), Layer (Z), m (A), n (A) (with U: other/unknown; L: unlimited).",
@@ -123,10 +130,7 @@ dataset_stem__expected_messages = [
             "§2.6.2 depth:comment should be a non-empty string",
             "§2.6.2 institution global attribute should be a non-empty string",
             "§2.6.2 comment global attribute should be a non-empty string",
-            "units (None) attribute of 'LayerInterf' must be a string compatible with UDUNITS",
-            "units (None) attribute of 'time_bounds' must be a string compatible with UDUNITS",
-            "units (None) attribute of 'Layer' must be a string compatible with UDUNITS",
-            "units for variable area must be convertible to m2 currently they are degrees2",
+            'Units "degrees2" for variable area must be convertible to canonical units "m2"',
             "k: vertical coordinates not defining pressure must include a positive attribute that is either 'up' or 'down'",
             "grid_longitude has no coordinate associated with a variable identified as true latitude/longitude; its coordinate variable should also share a subset of grid_longitude's dimensions",
             "grid_latitude has no coordinate associated with a variable identified as true latitude/longitude; its coordinate variable should also share a subset of grid_latitude's dimensions",
@@ -150,7 +154,7 @@ dataset_stem__expected_messages = [
         "glcfs",
         [
             # TODO: referenced/relative time is treated like time units
-            "units for variable time_offset must be convertible to s currently they are hours since 2016-01-01T12:00:00Z",
+            'Units "hours since 2016-01-01T12:00:00Z" for variable time_offset must be convertible to canonical units "s"',
             "standard_name cloud_cover is not defined in Standard Name Table v{}".format(
                 std_names._version
             ),
@@ -198,11 +202,6 @@ dataset_stem__expected_messages = [
             "w'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 ocean_time (T), s_w (Z), eta_rho (A), xi_rho (A) (with U: other/unknown; L: unlimited).",
             "zeta'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 ocean_time (T), eta_rho (A), xi_rho (A) (with U: other/unknown; L: unlimited).",
             '§2.6.1 Conventions global attribute does not contain "CF-1.8"',
-            "units (None) attribute of 's_w' must be a string compatible with UDUNITS",
-            "units (None) attribute of 's_rho' must be a string compatible with UDUNITS",
-            "units (None) attribute of 'Cs_w' must be a string compatible with UDUNITS",
-            "units (None) attribute of 'user' must be a string compatible with UDUNITS",
-            "units (None) attribute of 'Cs_r' must be a string compatible with UDUNITS",
             "CF recommends latitude variable 'lat_rho' to use units degrees_north",
             "CF recommends latitude variable 'lat_u' to use units degrees_north",
             "CF recommends latitude variable 'lat_v' to use units degrees_north",
diff --git a/compliance_checker/tests/test_protocols.py b/compliance_checker/tests/test_protocols.py
index 999c8d9..5143e26 100644
--- a/compliance_checker/tests/test_protocols.py
+++ b/compliance_checker/tests/test_protocols.py
@@ -36,7 +36,7 @@ class TestProtocols(TestCase):
         """
         Tests that a connection can be made to Hyrax
         """
-        url = "http://test.opendap.org:8080/opendap/ioos/mday_joinExist.ncml"
+        url = "http://ingria.coas.oregonstate.edu/opendap/hyrax/aggregated/ocean_time_aggregation.ncml"
         cs = CheckSuite()
         ds = cs.load_dataset(url)
         assert ds is not None
@@ -57,7 +57,7 @@ class TestProtocols(TestCase):
         """
         Tests that a connection can be made to an SOS endpoint
         """
-        url = "https://thredds.aoos.org/thredds/sos/aoos/cruises/ecofoci/2dy12.nc"
+        url = "https://data.oceansmap.com/thredds/sos/caricoos_ag/VIA/VIA.ncml"
         cs = CheckSuite()
         ds = cs.load_dataset(url)
         assert ds is not None