Repository: ioos/compliance-checker
Pull request #640 (2019-04-11 15:11:15)

Quick Improvements 2019-04-11
Addresses a few quick issues related to comparing data values to metadata, message improvements.

acdd.py:
  - check_lat_extents(), check_lon_extents(), and _check_total_z_extents() -- all tried to compare
    data values to metadata values, which may have been passed as strings. numpy copmarision would
    subsequently fail; fixed by casting the potential string values as floats (Issue #639)
  - check_coverage_content_type() was outputting the string "coverage_content" instead of
    "coverage_content_type"; inserted the correct string (Issue #637)

cf.py:
  - _check_formula_terms() was outputting a message about missing variables, but did not include
    the variables it was referring to. Fixed by adding a format field. (Issue #636)
  - check_grid_coordinates() message was a little ambiguous -- added some context, as well
    as a message for variables with missing coordinate variables (Issue #638)


Patch:
diff --git a/README.md b/README.md
index 257722b..a1ef3e5 100644
--- a/README.md
+++ b/README.md
@@ -24,7 +24,7 @@ It currently supports the following sources and standards:
 | [ACDD (1.1, 1.3)](http://wiki.esipfed.org/index.php/Attribute_Convention_for_Data_Discovery_1-3)                                    | Built-in                                                          | X                | -                               |
 | [CF (1.6)](http://cfconventions.org/cf-conventions/v1.6.0/cf-conventions.html)                                                      | Built-in                                                          | X                | -                               |
 | IOOS SOS                                                                                                                            | Built-in                                                          | -                | GetCapabilities, DescribeSensor |
-| [IOOS (1.1)](https://ioos.github.io/ioos-metadata/ioos-metadata-profile-v1-1.html#ioos-netcdf-metadata-profile-attributes) | Built-in                                                          | X                | -                               |
+| [IOOS (1.1)](https://ioos.github.io/ioos-netcdf/ioos-netcdf-metadata-description-v1-1.html#ioos-netcdf-metadata-profile-attributes) | Built-in                                                          | X                | -                               |
 | [Glider DAC](https://github.com/ioos/ioosngdac/wiki/NGDAC-NetCDF-File-Format-Version-2)                                             | [ioos/cc-plugin-glider](https://github.com/ioos/cc-plugin-glider) | X                | -                               |
 | [NCEI (1.1, 2.0)](https://www.nodc.noaa.gov/data/formats/netcdf/v2.0/)                                                              | [ioos/cc-plugin-ncei](https://github.com/ioos/cc-plugin-ncei)     | X                | -                               |
 
diff --git a/compliance_checker/acdd.py b/compliance_checker/acdd.py
index 831a442..ae2ac29 100644
--- a/compliance_checker/acdd.py
+++ b/compliance_checker/acdd.py
@@ -223,11 +223,23 @@ class ACDDBaseCheck(BaseCheck):
 
         :param netCDF4.Dataset ds: An open netCDF dataset
         '''
-        if not (hasattr(ds, 'geospatial_lat_min') and hasattr(ds, 'geospatial_lat_max')):
-            return
 
-        lat_min = ds.geospatial_lat_min
-        lat_max = ds.geospatial_lat_max
+        if not (hasattr(ds, 'geospatial_lat_min') or hasattr(ds, 'geospatial_lat_max')):
+            return Result(BaseCheck.MEDIUM,
+                          False,
+                          'geospatial_lat_extents_match',
+                          ['geospatial_lat_min/max attribute not found, CF-1.6 spec chapter 4.1'])
+
+        try: # type cast
+            lat_min = float(ds.geospatial_lat_min)
+            lat_max = float(ds.geospatial_lat_max)
+        except ValueError:
+            return Result(BaseCheck.MEDIUM,
+                          False,
+                          'geospatial_lat_extents_match',
+                          ['Could not convert one of geospatial_lat_min ({}) or max ({}) to float see CF-1.6 spec chapter 4.1'
+                          ''.format(ds.geospatial_lat_min, ds.geospatial_lat_max)])
+            
 
         # identify lat var(s) as per CF 4.1
         lat_vars = {}       # var -> number of criteria passed
@@ -289,11 +301,22 @@ class ACDDBaseCheck(BaseCheck):
 
         :param netCDF4.Dataset ds: An open netCDF dataset
         '''
+
         if not (hasattr(ds, 'geospatial_lon_min') and hasattr(ds, 'geospatial_lon_max')):
-            return
+            return Result(BaseCheck.MEDIUM,
+                          False,
+                          'geospatial_lon_extents_match',
+                          ['geospatial_lon_min/max attribute not found, CF-1.6 spec chapter 4.1'])
 
-        lon_min = ds.geospatial_lon_min
-        lon_max = ds.geospatial_lon_max
+        try: # type cast
+            lon_min = float(ds.geospatial_lon_min)
+            lon_max = float(ds.geospatial_lon_max)
+        except ValueError:
+            return Result(BaseCheck.MEDIUM,
+                          False,
+                          'geospatial_lon_extents_match',
+                          ['Could not convert one of geospatial_lon_min ({}) or max ({}) to float see CF-1.6 spec chapter 4.1'
+                          ''.format(ds.geospatial_lon_min, ds.geospatial_lon_max)])
 
         # identify lon var(s) as per CF 4.2
         lon_vars = {}       # var -> number of criteria passed
@@ -377,8 +400,8 @@ class ACDDBaseCheck(BaseCheck):
         :param netCDF4.Dataset ds: An open netCDF dataset
         :param str z_variable: Name of the variable representing the Z-Axis
         '''
-        vert_min = ds.geospatial_vertical_min
-        vert_max = ds.geospatial_vertical_max
+        vert_min = float(ds.geospatial_vertical_min)
+        vert_max = float(ds.geospatial_vertical_max)
         msgs = []
         total = 2
 
@@ -698,7 +721,7 @@ class ACDD1_3Check(ACDDNCCheck):
                             'coverage_content_type', None)
             check = ctype is not None
             if not check:
-                msgs.append("coverage_content")
+                msgs.append("coverage_content_type")
                 results.append(Result(BaseCheck.HIGH, check,
                                       self._var_header.format(variable), msgs))
                 continue
diff --git a/compliance_checker/cf/cf.py b/compliance_checker/cf/cf.py
index a71f343..1edf806 100644
--- a/compliance_checker/cf/cf.py
+++ b/compliance_checker/cf/cf.py
@@ -1843,9 +1843,9 @@ class CFBaseCheck(BaseCheck):
         # get the variables named in the formula terms and check if any
         # are not present in the dataset
         missing_vars = sorted(set(m[1] for m in matches) - set(ds.variables))
-        missing_fmt = "The following variable(s) referenced in formula_terms are not present in the dataset: {}".format(coord)
+        missing_fmt = "The following variable(s) referenced in {}:formula_terms are not present in the dataset: {}"
         valid_formula_terms.assert_true(len(missing_vars) == 0,
-                                    missing_fmt.format(', '.join(missing_vars)))
+                                    missing_fmt.format(coord, ', '.join(missing_vars)))
         # try to reconstruct formula_terms by adding space in between the regex
         # matches.  If it doesn't exactly match the original, the formatting
         # of the attribute is incorrect
@@ -2206,29 +2206,42 @@ class CFBaseCheck(BaseCheck):
             # contents of the `coordinates` attribute only.
             axis_map = cfutil.get_axis_map(ds, variable)
 
+            msg = '{}\'s coordinate variable "{}" is not one of the variables identifying true '+\
+                  'latitude/longitude and its dimensions are not a subset of {}\'s dimensions'
+
+            alt = '{} has no coordinate associated with a variable identified as true latitude/longitude; '+\
+                  'its coordinate variable should also share a subset of {}\'s dimensions'
+
             # Make sure we can find latitude and its dimensions are a subset
+            _lat = None 
             found_lat = False
             for lat in axis_map['Y']:
+                _lat = lat
                 is_subset_dims = set(ds.variables[lat].dimensions).issubset(dimensions)
 
                 if is_subset_dims and lat in latitudes:
                     found_lat = True
                     break
-            has_coords.assert_true(found_lat,
-                                   '{} is not associated with a coordinate defining true latitude '
-                                   'and sharing a subset of dimensions'.format(variable))
+            if _lat:
+                has_coords.assert_true(found_lat, msg.format(variable, _lat, variable))
+            else:
+                has_coords.assert_true(found_lat, alt.format(variable, variable))
+                
 
             # Make sure we can find longitude and its dimensions are a subset
+            _lon = None 
             found_lon = False
             for lon in axis_map['X']:
+                _lon = lon
                 is_subset_dims = set(ds.variables[lon].dimensions).issubset(dimensions)
 
                 if is_subset_dims and lon in longitudes:
                     found_lon = True
                     break
-            has_coords.assert_true(found_lon,
-                                   '{} is not associated with a coordinate defining true longitude '
-                                   'and sharing a subset of dimensions'.format(variable))
+            if _lon:
+                has_coords.assert_true(found_lon, msg.format(variable, _lon, variable))
+            else:
+                has_coords.assert_true(found_lon, alt.format(variable, variable))
 
             ret_val.append(has_coords.to_result())
         return ret_val
diff --git a/compliance_checker/data/templates/ccheck.html.j2 b/compliance_checker/data/templates/ccheck.html.j2
index b398821..2a51aef 100644
--- a/compliance_checker/data/templates/ccheck.html.j2
+++ b/compliance_checker/data/templates/ccheck.html.j2
@@ -1,55 +1,13 @@
 <!-- Template for the HTML markup for the results of a single test -->
-
-<style>
-
-/* ------------------------- */
-/* ----- Table Styling ----- */
-/* ------------------------- */
-
-table {
-  font-family: helvetica, sans-serif;
-  border-collapse: collapse;
-  width: 100%;
-}
-
-th{
-    text-align: left;
-    padding: 12 px;
-}
-
-td{
-  /*border-top: 1px solid #dddddd;
-  border-bottom: 1px solid #dddddd;
-  border-left: 1px solid #dddddd;
-  border-right: 1px solid #dddddd;*/
-  font-size: 14px;
-  text-align: left;
-  padding: 8px;
-}
-
-td:first-child{
-  font-style: italic;
-}
-
-/* ------------------------ */
-/* ----- List styling ----- */
-/* ------------------------ */
-
-li{
-  line-height: 14px;
-}
-
-</style>
-
 <div class="row">
   <div class="col-md-12">
     <div class="page-header">
-      <h2 align="center">IOOS Compliance Checker Report</h2>
-      <p align="center">For dataset {{source_name}}</p>
+      <h2>IOOS Compliance Checker Report</h2>
+      <p>For dataset {{source_name}}</p>
     </div>
   </div>
   <div class="col-md-12">
-    <h3 align="center">
+    <h3>
       {% if cc_url %}
         <a href={{cc_url}} target="_blank">{{testname}}</a>
       {% else %}
@@ -58,18 +16,9 @@ li{
     </h3>
     <!--<a href={{cc_url}}>Reference Page</a>-->
   </div>
-
-  <!-- -------------- -->
-  <!-- RESULTS TABLES -->
-  <!-- -------------- -->
-
   <div class="col-md-12">
-    <!--<h4>Corrective Actions</h4>-->
-    <h4 align="center">Corrective Actions</h4>
+    <h4>Corrective Actions</h4>
     <div class="col-md-12">
-
-      <!-- High Priority -->
-
       <div class="table-collapse">
         <a data-target="high-priority-table" href="#"> {{scoreheader.get(3)}} <i class="glyphicon glyphicon-collapse-up"></i></a>
       </div>
@@ -77,7 +26,6 @@ li{
       <div class="failures">
         | <span class="label label-danger label-as-badge">{{ high_count }}</span>
       </div>
-
       <div class="high-priority-table collapse in">
         <table class="table">
           <thead>
@@ -89,31 +37,17 @@ li{
           <tbody>
             {% for result in all_priorities -%}
               {% if result['weight'] == 3 -%}
-
-
+                {% for msg in result['msgs']-%}
                   <tr>
-                    {% if result["msgs"]|length > 0 -%}
-                      <td>{{result['name']}}</td>
-                      <td>
-                          {% for msg in result['msgs'] -%}
-
-                          <!-- bulleted list inside the table row -->
-                        <ul>
-                          <li>{{msg}}</li>
-                        </ul>
-
-                        {% endfor -%}
-                    {% endif -%}
-                    </td>
+                    <td>{{result['name']}}</td>
+                    <td>{{msg}}</td>
                   </tr>
-
-
+                {% endfor -%}
               {% endif -%}
             {% endfor -%}
           </tbody>
         </table>
       </div> <!-- .high-priority-table -->
-
       {% else -%}
       <div class="failures">
         | <span class="label label-success label-as-badge">{{ high_count }}</span>
@@ -130,9 +64,6 @@ li{
       </div>
       {% endif -%}
     </div> <!-- .col-md-12 -->
-
-    <!-- Medium Priority -->
-
     <div class="col-md-12">
       {% if medium_count -%}
       <div class="table-collapse">
@@ -149,39 +80,22 @@ li{
               <th class="ccorrection">Reasoning</th>
             </tr>
           </thead>
-
           <tbody>
             {% for result in all_priorities -%}
               {% if result['weight'] == 2 -%}
-
+                {% for msg in result['msgs']-%}
                   <tr>
-                    {% if result["msgs"]|length -%}
-                      <td>{{result['name']}}</td>
-                      <td>
-                          {% for msg in result['msgs'] -%}
-
-                          <!-- bulleted list inside the table row -->
-                        <ul>
-                          <li>{{msg}}</li>
-                        </ul>
-
-                        {% endfor -%}
-                    {% endif -%}
-                    </td>
+                    <td>{{result['name']}}</td>
+                    <td>{{msg}}</td>
                   </tr>
-
-
+                {% endfor -%}
               {% endif -%}
             {% endfor -%}
           </tbody>
-
         </table>
       </div> <!-- .medium-priority-table -->
       {% endif -%}
     </div> <!-- .col-md-12 -->
-
-    <!-- Low Priority -->
-    
     <div class="col-md-12">
       {% if low_count -%}
       <div class="table-collapse">
@@ -198,36 +112,21 @@ li{
               <th class="ccorrection">Reasoning</th>
             </tr>
           </thead>
-
           <tbody>
             {% for result in all_priorities -%}
               {% if result['weight'] == 1 -%}
-
+                {% for msg in result['msgs']-%}
                   <tr>
-                    {% if result["msgs"]|length -%}
-                      <td>{{result['name']}}</td>
-                      <td>
-                          {% for msg in result['msgs'] -%}
-
-                          <!-- bulleted list inside the table row -->
-                        <ul>
-                          <li>{{msg}}</li>
-                        </ul>
-
-                        {% endfor -%}
-                    {% endif -%}
-                    </td>
+                    <td>{{result['name']}}</td>
+                    <td>{{msg}}</td>
                   </tr>
-
-
+                {% endfor -%}
               {% endif -%}
             {% endfor -%}
           </tbody>
-
         </table>
       </div> <!-- .low-priority-table -->
       {% endif -%}
     </div> <!-- .col-md-12 -->
-
   </div> <!-- .col-md-12 -->
 </div> <!-- .row -->
diff --git a/compliance_checker/data/templates/ccheck_wrapper.html.j2 b/compliance_checker/data/templates/ccheck_wrapper.html.j2
index 18d1761..af98974 100644
--- a/compliance_checker/data/templates/ccheck_wrapper.html.j2
+++ b/compliance_checker/data/templates/ccheck_wrapper.html.j2
@@ -199,7 +199,6 @@ thead .ccorrection {
     <script type="text/javascript">
 $(document).ready(function() {
     $('.high-priority-table').collapse({toggle: false});
-
     $('.table-collapse').click(function(e) {
       e.preventDefault();
       // Find table to toggle within this check
@@ -215,23 +214,6 @@ $(document).ready(function() {
         glyph.addClass('glyphicon-collapse-up');
       }
     });
-
-
-    $('.table-collapse i.glyphicon').click(function(e) {
-      e.preventDefault();
-      // Find table to toggle within this check
-      var checkWrapper = $(e.target).closest(".check_wrapper");
-      $(checkWrapper).find('.' + $(e.target.parentElement).data('target')).collapse('toggle');
-
-      var glyph = $(e.target.parentElement).find('.glyphicon');
-      if(glyph.hasClass('glyphicon-collapse-up')) {
-        glyph.removeClass('glyphicon-collapse-up');
-        glyph.addClass('glyphicon-collapse-down');
-      } else {
-        glyph.removeClass('glyphicon-collapse-down');
-        glyph.addClass('glyphicon-collapse-up');
-      }
-    });
 });
     </script>
   </body>
diff --git a/compliance_checker/ioos.py b/compliance_checker/ioos.py
index 07fb51d..0027888 100644
--- a/compliance_checker/ioos.py
+++ b/compliance_checker/ioos.py
@@ -13,7 +13,7 @@ class IOOSBaseCheck(BaseCheck):
     _cc_spec = 'ioos'
     _cc_spec_version = '0.1'
     _cc_description = 'IOOS Inventory Metadata'
-    _cc_url = 'https://ioos.github.io/ioos-metadata/ioos-metadata-profile-v1-1.html#ioos-netcdf-metadata-profile-attributes'
+    _cc_url = 'https://ioos.github.io/ioos-netcdf/ioos-netcdf-metadata-description-v1-1.html#ioos-netcdf-metadata-profile-attributes'
     _cc_display_headers = {
         3: 'Highly Recommended',
         2: 'Recommended',
@@ -194,16 +194,16 @@ class IOOS0_1Check(IOOSNCCheck):
 
 class IOOS1_1Check(IOOSNCCheck):
     '''
-    Compliance checker implementation of IOOS Metadata Profile, Version 1.1
+    Compliance checker implementation of IOOS Metadata Profile for NetCDF, Version 1.1
 
     Related links:
-    https://ioos.github.io/ioos-metadata/ioos-metadata-profile-v1-1.html#ioos-netcdf-metadata-profile-attributes
+    https://ioos.github.io/ioos-netcdf/ioos-netcdf-metadata-description-v1-1.html#ioos-netcdf-metadata-profile-attributes
     https://github.com/ioos/compliance-checker/issues/69
     https://github.com/ioos/compliance-checker/issues/358
     '''
     _cc_spec_version = '1.1'
-    _cc_description = 'IOOS Metadata Profile, Version 1.1'
-    _cc_url = 'https://ioos.github.io/ioos-metadata/ioos-metadata-profile-v1-1.html#ioos-netcdf-metadata-profile-attributes'
+    _cc_description = 'IOOS Metadata Profile for NetCDF, Version 1.1'
+    _cc_url = 'https://ioos.github.io/ioos-netcdf/ioos-netcdf-metadata-description-v1-1.html#ioos-netcdf-metadata-profile-attributes'
     register_checker = True
 
     def __init__(self):
diff --git a/compliance_checker/protocols/opendap.py b/compliance_checker/protocols/opendap.py
index fb4fcd5..df6a829 100644
--- a/compliance_checker/protocols/opendap.py
+++ b/compliance_checker/protocols/opendap.py
@@ -14,10 +14,7 @@ def is_opendap(url):
     :param str url: URL for a remote OPeNDAP endpoint
     '''
     # If the server replies to a Data Attribute Structure request
-    if url.endswith('#fillmismatch'):
-        das_url = url.replace('#fillmismatch', '.das')
-    else:
-        das_url = url + '.das'
+    das_url = url + '.das'
     response = requests.get(das_url, allow_redirects=True)
     if 'xdods-server' in response.headers:
         return True
diff --git a/setup.py b/setup.py
index beaba98..21e8a1e 100644
--- a/setup.py
+++ b/setup.py
@@ -1,10 +1,9 @@
 from __future__ import with_statement
 from setuptools import setup, find_packages
-import io
 import versioneer
 
 def readme():
-    with io.open('README.md', encoding="utf-8") as f:
+    with open('README.md') as f:
         return f.read()
 
 def pip_requirements(fname='requirements.txt'):


Test patch:
diff --git a/compliance_checker/tests/helpers.py b/compliance_checker/tests/helpers.py
index 0742aaf..4a77cfd 100644
--- a/compliance_checker/tests/helpers.py
+++ b/compliance_checker/tests/helpers.py
@@ -27,6 +27,12 @@ class MockTimeSeries(MockNetCDF):
         for v in ('time', 'lon', 'lat', 'depth'):
             self.createVariable(v, 'd', ('time',))
 
+        # give some applicable units
+        self.variables["time"].units  = "seconds since 2019-04-11T00:00:00"
+        self.variables["lat"].units   = "degree_north"
+        self.variables["lon"].units   = "degree_east"
+        self.variables["depth"].units = "meters"
+
 
 class MockVariable(object):
     '''
diff --git a/compliance_checker/tests/test_acdd.py b/compliance_checker/tests/test_acdd.py
index ff6dcfc..bdde501 100644
--- a/compliance_checker/tests/test_acdd.py
+++ b/compliance_checker/tests/test_acdd.py
@@ -1,8 +1,10 @@
 from compliance_checker.acdd import ACDD1_1Check, ACDD1_3Check
+from compliance_checker.tests.helpers import MockTimeSeries, MockVariable
 from compliance_checker.tests.resources import STATIC_FILES
 from compliance_checker.tests import BaseTestCase
 from netCDF4 import Dataset
 import os
+import numpy as np
 
 
 def to_singleton_var(l):
@@ -13,7 +15,6 @@ def to_singleton_var(l):
     return [x[0] if hasattr(x, '__iter__') and not isinstance(x, str) else x
             for x in l]
 
-
 def check_varset_nonintersect(group0, group1):
     '''
     Returns true if both groups contain the same elements, regardless of
@@ -433,3 +434,82 @@ class TestACDD1_3(BaseTestCase):
         empty_ds.time_coverage_end = '20160102T000000-1000'
         result = self.acdd.check_time_extents(empty_ds)
         self.assert_result_is_good(result)
+
+    def test_check_lat_extents(self):
+        """Test the check_lat_extents() method behaves expectedly"""
+
+        # create dataset using MockDataset, give it lat/lon dimensions
+        ds = MockTimeSeries()
+        ds.variables["lat"][:] = np.linspace(-135., -130., num=500) # arbitrary, but matches time dim size
+
+        # test no values, expect failure
+        result = self.acdd.check_lat_extents(ds)
+        self.assert_result_is_bad(result)
+
+        # give integer geospatial_lat_max/min, test
+        ds.setncattr("geospatial_lat_min", -135)
+        ds.setncattr("geospatial_lat_max", -130)
+
+        result = self.acdd.check_lat_extents(ds)
+        self.assert_result_is_good(result)
+
+        # give float geospatial_lat_min/max, test
+        ds.setncattr("geospatial_lat_min", -135.)
+        ds.setncattr("geospatial_lat_max", -130.)
+        
+        result = self.acdd.check_lat_extents(ds)
+        self.assert_result_is_good(result)
+
+        # give string (in number-form), test
+        ds.setncattr("geospatial_lat_min", "-135.")
+        ds.setncattr("geospatial_lat_max", "-130.")
+        
+        result = self.acdd.check_lat_extents(ds)
+        self.assert_result_is_good(result)
+
+        # give garbage string -- expect failure
+        ds.setncattr("geospatial_lat_min", "bad")
+        ds.setncattr("geospatial_lat_max", "val")
+        
+        result = self.acdd.check_lat_extents(ds)
+        self.assert_result_is_bad(result)
+
+    def test_check_lon_extents(self):
+        """Test the check_lon_extents() method behaves expectedly"""
+
+        # create dataset using MockDataset, give it lat/lon dimensions
+        ds = MockTimeSeries()
+        ds.variables["lon"][:] = np.linspace(65., 67., num=500) 
+
+        # test no values, expect failure
+        #result = self.acdd.check_lon_extents(ds)
+        #self.assert_result_is_bad(result)
+
+        # give integer geospatial_lon_max/min, test
+        ds.setncattr("geospatial_lon_min", 65)
+        ds.setncattr("geospatial_lon_max", 67)
+
+        result = self.acdd.check_lon_extents(ds)
+        self.assert_result_is_good(result)
+
+        # give float geospatial_lon_min/max, test
+        ds.setncattr("geospatial_lon_min", 65.)
+        ds.setncattr("geospatial_lon_max", 67.)
+        
+        result = self.acdd.check_lon_extents(ds)
+        self.assert_result_is_good(result)
+
+        # give string (in number-form), test
+        ds.setncattr("geospatial_lon_min", "65.")
+        ds.setncattr("geospatial_lon_max", "67.")
+        
+        result = self.acdd.check_lon_extents(ds)
+        self.assert_result_is_good(result)
+
+        # give garbage string -- expect failure
+        ds.setncattr("geospatial_lon_min", "bad")
+        ds.setncattr("geospatial_lon_max", "val")
+        
+        result = self.acdd.check_lon_extents(ds)
+        self.assert_result_is_bad(result)
+