Merge pull request #19 from ISI-MIP/ruff

Add ruff and pre-commit config and fix issues

.pre-commit-config.yaml

@@ -0,0 +1,16 @@
+repos:
+  - repo: meta
+    hooks:
+      - id: check-hooks-apply
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.4.0
+    hooks:
+      - id: check-ast
+      - id: end-of-file-fixer
+      - id: trailing-whitespace
+      - id: debug-statements
+  - repo: https://github.com/charliermarsh/ruff-pre-commit
+    rev: v0.0.284
+    hooks:
+      - id: ruff
+        args: [--fix, --exit-non-zero-on-fix]

isimip_qc/__init__.py

@@ -1 +1 @@
-VERSION = __version__ = '3.2.5'
+VERSION = __version__ = '3.3.0'

isimip_qc/checks/3d.py

@@ -1,6 +1,8 @@
 from isimip_qc.config import settings
+
 from ..exceptions import FileCritical
 
+
 def check_3d(file):
     crop = file.specifiers.get('crop')
     irrigation = file.specifiers.get('irrigation')
@@ -27,15 +29,18 @@ def check_3d(file):
     try:
         variable = file.dataset.variables.get(file.variable_name)
         if variable is None:
-            file.critical('Variable "%s" from file name not found inside the file! Check NetCDF header. Stopping tool!', file.variable_name)
+            file.critical('Variable "%s" from file name not found inside the file! Check NetCDF header. Stopping tool!',
+                          file.variable_name)
             raise SystemExit(1)
-    except AttributeError:
-        file.critical('Variable "%s" from file name not found inside the file! Check NetCDF header. Stopping tool!', file.variable_name)
-        raise SystemExit(1)
+    except AttributeError as e:
+        file.critical('Variable "%s" from file name not found inside the file! Check NetCDF header. Stopping tool!',
+                      file.variable_name)
+        raise SystemExit(1) from e
 
     definition = settings.DEFINITIONS.get('variable', {}).get(file.specifiers.get('variable'))
     if definition is None:
-        raise FileCritical(file,'Variable %s not defined for sector %s. skipping...', file.variable_name, settings.SECTOR)
+        raise FileCritical(file, 'Variable %s not defined for sector %s. skipping...',
+                           file.variable_name, settings.SECTOR)
 
     # check for number of variable dependencies
 
@@ -47,8 +52,10 @@ def check_3d(file):
         settings_dim_len = 3
 
     if file_dim_len != settings_dim_len:
-        file.critical('Dimension missing (%s found, %s expected). Declare as %s%s. ',
-                      file_dim_len, settings_dim_len, file.variable_name, str(definition['dimensions']).replace('\'',''))
+        file.critical(
+            'Dimension missing (%s found, %s expected). Declare as %s%s. ',
+            file_dim_len, settings_dim_len, file.variable_name, str(definition['dimensions']).replace('\'', '')
+        )
 
     # detect 2d or 3d data
     file.is_time_fixed = False
@@ -66,7 +73,9 @@ def check_3d(file):
             for pos in range(0, file_dim_len):
                 if variable.dimensions[pos] not in ['time', 'lat', 'lon']:
                     break
-            file.critical('Variable "%s" has "time", "lat" or "lon" as second dependeny. Depencency order must be [time, %s, lat, lon]. "time" is first always.', file.variable_name, variable.dimensions[pos])
+            file.critical('Variable "%s" has "time", "lat" or "lon" as second dependeny.'
+                          ' Depencency order must be [time, %s, lat, lon]. "time" is first always.',
+                          file.variable_name, variable.dimensions[pos])
             file.dim_vertical = variable.dimensions[pos]
         else:
             file.dim_vertical = variable.dimensions[1]

isimip_qc/checks/__init__.py

@@ -15,7 +15,7 @@
             parts = file_path.relative_to(checks_dir).with_suffix('').parts
             module_name = 'isimip_qc.checks.{}'.format('.'.join(parts))
             module = importlib.import_module(module_name)
-            functions += [function for function in inspect.getmembers(module, inspect.isfunction)]
+            functions += list(inspect.getmembers(module, inspect.isfunction))
 
 # find checks
 checks = [function[1] for function in functions if function[0].startswith('check_')]

isimip_qc/checks/attributes.py

@@ -1,10 +1,10 @@
 import uuid
-from email.utils import parseaddr
 from datetime import datetime
+from email.utils import parseaddr
 
 from .. import __version__
 from ..config import settings
-from ..fixes import fix_set_global_attr, fix_remove_global_attr
+from ..fixes import fix_remove_global_attr, fix_set_global_attr
 
 
 def check_isimip_id(file):
@@ -47,11 +47,13 @@ def check_isimip_protocol_version(file):
             file.info('Global attribute "isimip_protocol_version" matches current protocol version (%s).',
                       version)
         else:
-            file.info('Global attribute "isimip_protocol_version" (%s) does not match tool current protocol version (%s).',
-                      version, protocol_version, fix={
-                          'func': fix_set_global_attr,
-                          'args': (file, 'isimip_protocol_version', protocol_version)
-                      })
+            file.info(
+                'Global attribute "isimip_protocol_version" (%s) does not match tool current protocol version (%s).',
+                version, protocol_version, fix={
+                    'func': fix_set_global_attr,
+                    'args': (file, 'isimip_protocol_version', protocol_version)
+                }
+            )
     except AttributeError:
         file.info('Global attribute "isimip_protocol_version" not yet set.',
                   fix={

isimip_qc/checks/dataset.py

@@ -1,5 +1,4 @@
-from ..fixes import (fix_remove_variable_attr, fix_rename_dimension,
-                     fix_rename_variable, fix_rename_variable_attr)
+from ..fixes import fix_remove_variable_attr, fix_rename_dimension, fix_rename_variable, fix_rename_variable_attr
 
 
 def check_data_model(file):
@@ -24,7 +23,8 @@ def check_zip(file):
         if zlib:
             complevel = variable.filters().get('complevel')
             if complevel < 4:
-                file.warn('Variable "%s" compression level is "%s". Should be >= 5.', file.variable_name, complevel, fix_datamodel=True)
+                file.warn('Variable "%s" compression level is "%s". Should be >= 5.',
+                          file.variable_name, complevel, fix_datamodel=True)
             else:
                 file.info('Variable "%s" compression level looks good (%s)', file.variable_name, complevel)
         else:
@@ -54,7 +54,9 @@ def check_lower_case(file):
 
         for attr in variable.__dict__:
             if attr not in ['_FillValue']:
-                if attr not in ['axis', 'standard_name', 'long_name', 'calendar', 'missing_value', 'units', 'comment', 'enteric_infection', 'description', 'unit_conversion_info', 'positive', 'bounds', 'classes']:
+                if attr not in ['axis', 'standard_name', 'long_name', 'calendar', 'missing_value',
+                                'units', 'comment', 'enteric_infection', 'description', 'unit_conversion_info',
+                                'positive', 'bounds', 'classes']:
                     file.warn('Attribute "%s" for variable "%s" is not needed.', attr, variable_name, fix={
                         'func': fix_remove_variable_attr,
                         'args': (file, variable_name, attr)

isimip_qc/checks/dimensions.py

@@ -21,11 +21,12 @@ def check_lon_dimension(file):
                 grid_info = settings.DEFINITIONS['climate_forcing'].get(climate_forcing)['grid']['lon_size']
                 try:
                     lon_size = grid_info[sens_scenario]
-                except:
+                except KeyError:
                     lon_size = grid_info['default']
 
             if lon_size != file.dataset.dimensions.get('lon').size:
-                file.warn('Unexpected number of longitudes found (%s). Should be %s', file.dataset.dimensions.get('lon').size, lon_size)
+                file.warn('Unexpected number of longitudes found (%s). Should be %s',
+                          file.dataset.dimensions.get('lon').size, lon_size)
             else:
                 file.info('%s longitudes defined.', lon_size)
 
@@ -50,11 +51,12 @@ def check_lat_dimension(file):
                 grid_info = settings.DEFINITIONS['climate_forcing'].get(climate_forcing)['grid']['lat_size']
                 try:
                     lat_size = grid_info[sens_scenario]
-                except:
+                except KeyError:
                     lat_size = grid_info['default']
 
             if lat_size != file.dataset.dimensions.get('lat').size:
-                file.warn('Unexpected number of latitudes found (%s). Should be %s', file.dataset.dimensions.get('lat').size, lat_size)
+                file.warn('Unexpected number of latitudes found (%s). Should be %s',
+                          file.dataset.dimensions.get('lat').size, lat_size)
             else:
                 file.info('%s latitudes defined.', lat_size)
 
@@ -67,7 +69,8 @@ def check_time_dimension(file):
 def check_depth_dimension(file):
     if file.is_3d:
         if file.dataset.dimensions.get(file.dim_vertical) is None:
-            file.error('Valid 4th dimension is missing. Should be of of [depth, bins]. Found "%s" instead.', file.dim_vertical)
+            file.error('Valid 4th dimension is missing. Should be of of [depth, bins]. Found "%s" instead.',
+                       file.dim_vertical)
 
 
 def check_dimensions(file):
@@ -77,17 +80,22 @@ def check_dimensions(file):
     dim_len = len(variable.dimensions)
     if file.is_time_fixed:
         if variable.dimensions[0] != 'lat' or variable.dimensions[1] != 'lon':
-            file.error('Dimension order for variable "%s" is %s. Should be ["lat", "lon"].', file.variable_name, variable.dimensions)
+            file.error('Dimension order for variable "%s" is %s. Should be ["lat", "lon"].',
+                       file.variable_name, variable.dimensions)
         else:
-            file.info('Dimensions for variable "%s" look good: %s.', file.variable_name, variable.dimensions)
+            file.info('Dimensions for variable "%s" look good: %s.',
+                      file.variable_name, variable.dimensions)
     elif file.is_2d:
         if variable.dimensions[0] != 'time' or variable.dimensions[1] != 'lat' or variable.dimensions[2] != 'lon':
-            file.error('Dimension order for variable "%s" is %s. Should be ["time", "lat", "lon"].', file.variable_name, variable.dimensions)
+            file.error('Dimension order for variable "%s" is %s. Should be ["time", "lat", "lon"].',
+                       file.variable_name, variable.dimensions)
         else:
-            file.info('Dimensions for variable "%s" look good: %s.', file.variable_name, variable.dimensions)
+            file.info('Dimensions for variable "%s" look good: %s.',
+                      file.variable_name, variable.dimensions)
     elif file.is_3d:
         if variable.dimensions[0] != 'time' or variable.dimensions[2] != 'lat' or variable.dimensions[3] != 'lon':
-            file.error('Dimension order for variable "%s" is %s. Should be ["time", "%s" , "lat", "lon"].', file.variable_name, variable.dimensions, file.dim_vertical)
+            file.error('Dimension order for variable "%s" is %s. Should be ["time", "%s" , "lat", "lon"].',
+                       file.variable_name, variable.dimensions, file.dim_vertical)
         else:
             file.info('Dimensions for variable "%s" look good.', file.variable_name)
     else:
@@ -97,7 +105,7 @@ def check_dimensions(file):
         dimension_definition = settings.DEFINITIONS['dimensions'].get(dimension_name)
 
         if dimension_definition:
-            # size of lat and lon are checked above 
+            # size of lat and lon are checked above
             if dimension_definition.get('specifier') not in ['lat', 'lon']:
                 size = dimension_definition.get('size')
                 if size and dimension.size != size:

isimip_qc/checks/experiments.py

@@ -1,5 +1,6 @@
-from ..utils.experiments import get_experiment
 from ..exceptions import FileCritical
+from ..utils.experiments import get_experiment
+
 
 def check_experiment(file):
     if file.specifiers.get("time_step") == "fixed":

isimip_qc/checks/variables/latlon.py

@@ -1,4 +1,5 @@
 import numpy as np
+
 from isimip_qc.config import settings
 from isimip_qc.fixes import fix_set_variable_attr
 
@@ -20,7 +21,8 @@ def check_latlon_variable(file):
             # check dtype
             dtypes = ['float32', 'float64']
             if var.dtype not in dtypes:
-                file.warn('Data type of "%s" is "%s". Should be float or double (one of %s).', variable, var.dtype, dtypes)
+                file.warn('Data type of "%s" is "%s". Should be float or double (one of %s).',
+                          variable, var.dtype, dtypes)
 
             # check axis
             axis = var_definition.get('axis')
@@ -40,43 +42,61 @@ def check_latlon_variable(file):
             standard_name = var_definition.get('standard_name')
             try:
                 if var.standard_name != standard_name:
-                    file.warn('"standard_name" attribute of "%s" is "%s". Should be "%s".', variable, var.standard_name, standard_name, fix={
+                    file.warn(
+                        '"standard_name" attribute of "%s" is "%s". Should be "%s".',
+                        variable, var.standard_name, standard_name, fix={
+                            'func': fix_set_variable_attr,
+                            'args': (file, variable, 'standard_name', standard_name)
+                        }
+                    )
+            except AttributeError:
+                file.warn(
+                    '"standard_name" attribute of "%s" is missing. Should be "%s".',
+                    variable, standard_name, fix={
                         'func': fix_set_variable_attr,
                         'args': (file, variable, 'standard_name', standard_name)
-                    })
-            except AttributeError:
-                file.warn('"standard_name" attribute of "%s" is missing. Should be "%s".', variable, standard_name, fix={
-                    'func': fix_set_variable_attr,
-                    'args': (file, variable, 'standard_name', standard_name)
-                })
+                    }
+                )
 
             # check long_name
             long_names = var_definition.get('long_names', [])
             try:
                 if var.long_name not in long_names:
-                    file.warn('"long_name" attribute of "%s" is %s". Should be in %s.', variable, var.long_name, long_names, fix={
+                    file.warn(
+                        '"long_name" attribute of "%s" is %s". Should be in %s.',
+                        variable, var.long_name, long_names, fix={
+                            'func': fix_set_variable_attr,
+                            'args': (file, variable, 'long_name', long_names[0])
+                        }
+                    )
+            except AttributeError:
+                file.warn(
+                    '"long_name" attribute of "%s" is missing. Should be "%s".',
+                    variable, long_names[0], fix={
                         'func': fix_set_variable_attr,
                         'args': (file, variable, 'long_name', long_names[0])
-                    })
-            except AttributeError:
-                file.warn('"long_name" attribute of "%s" is missing. Should be "%s".', variable, long_names[0], fix={
-                    'func': fix_set_variable_attr,
-                    'args': (file, variable, 'long_name', long_names[0])
-                })
+                    }
+                )
 
             # check units
             units = var_definition.get('units')
             try:
                 if var.units != units:
-                    file.warn('"units" attribute for "%s" is "%s". Should be "%s".', variable, var.units, units, fix={
+                    file.warn(
+                        '"units" attribute for "%s" is "%s". Should be "%s".',
+                        variable, var.units, units, fix={
+                            'func': fix_set_variable_attr,
+                            'args': (file, variable, 'units', units)
+                        }
+                    )
+            except AttributeError:
+                file.warn(
+                    '"units" attribute for "%s" is missing. Should be "%s".',
+                    variable, units, fix={
                         'func': fix_set_variable_attr,
                         'args': (file, variable, 'units', units)
-                    })
-            except AttributeError:
-                file.warn('"units" attribute for "%s" is missing. Should be "%s".', variable, units, fix={
-                    'func': fix_set_variable_attr,
-                    'args': (file, variable, 'units', units)
-                })
+                    }
+                )
 
             if settings.SECTOR not in ['marine-fishery_regional', 'water_regional', 'lakes_local', 'forestry']:
 
@@ -94,7 +114,7 @@ def check_latlon_variable(file):
                         elif var.name == 'lon':
                             minimum = grid_info['lon_min'][sens_scenario]
                             maximum = grid_info['lon_max'][sens_scenario]
-                    except:
+                    except KeyError:
                         if var.name == 'lat':
                             minimum = grid_info['lat_min']['default']
                             maximum = grid_info['lat_max']['default']
@@ -126,6 +146,7 @@ def check_latlon_variable(file):
                     lat_first = file.dataset.variables.get('lat')[0]
                     lat_last = file.dataset.variables.get('lat')[-1]
                     if lat_first < lat_last:
-                        file.warn('Latitudes in wrong order. Index should range from north to south. (found %s to %s)', lat_first, lat_last)
+                        file.warn('Latitudes in wrong order. Index should range from north to south. (found %s to %s)',
+                                  lat_first, lat_last)
                     else:
                         file.info('Latitude index order looks good (N to S).')