From 547c9c51b28c5f8afc94fb261891feede2ee31cb Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Wed, 13 Nov 2019 15:20:12 +0000 Subject: [PATCH 01/59] ported Javi's functionality from dry checks to mainstream to be used with differing cmor checks strictness levels --- esmvalcore/cmor/check.py | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 735ec1f09b..b0323e5274 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -57,10 +57,12 @@ def __init__(self, var_info, frequency=None, fail_on_error=False, + raise_exception=True, automatic_fixes=False): self._cube = cube self._failerr = fail_on_error + self._raise_exception = raise_exception self._errors = list() self._warnings = list() self._debug_messages = list() @@ -104,13 +106,16 @@ def check_metadata(self, logger=None): self.report_debug_messages(logger) self.report_warnings(logger) - self.report_errors() + self.report_errors(logger) if self.frequency != 'fx': self._add_auxiliar_time_coordinates() - return self._cube + if self.has_errors(): + return None + else: + return self._cube - def report_errors(self): + def report_errors(self, logger): """Report detected errors. Raises @@ -123,7 +128,9 @@ def report_errors(self): msg = 'There were errors in variable {}:\n{}\nin cube:\n{}' msg = msg.format(self._cube.var_name, '\n '.join(self._errors), self._cube) - raise CMORCheckError(msg) + if self._raise_exception: + raise CMORCheckError(msg) + logger.error(msg) def report_warnings(self, logger): """Report detected warnings to the given logger. @@ -179,7 +186,7 @@ def check_data(self, logger=None): self._check_coords_data() self.report_warnings(logger) - self.report_errors() + self.report_errors(logger) return self._cube def _check_fill_value(self): @@ -668,6 +675,7 @@ def _get_cmor_checker(table, short_name, frequency, fail_on_error=True, + raise_exception=True, automatic_fixes=False): """Get a CMOR checker/fixer.""" if table not in CMOR_TABLES: @@ -687,12 +695,14 @@ def _checker(cube): var_info, frequency=frequency, fail_on_error=fail_on_error, + raise_exception=raise_exception, automatic_fixes=automatic_fixes) return _checker -def cmor_check_metadata(cube, cmor_table, mip, short_name, frequency): +def cmor_check_metadata(cube, cmor_table, mip, + short_name, frequency, raise_exception): """Check if metadata conforms to variable's CMOR definiton. None of the checks at this step will force the cube to load the data. @@ -709,9 +719,14 @@ def cmor_check_metadata(cube, cmor_table, mip, short_name, frequency): Variable's short name. frequency: basestring Data frequency. + raise_exception: bool + Boolean operator that raises or not + the exception resulted from checker. """ - checker = _get_cmor_checker(cmor_table, mip, short_name, frequency) + checker = _get_cmor_checker(cmor_table, mip, + short_name, frequency, + raise_exception=raise_exception) checker(cube).check_metadata() return cube From b693bc08d0e1935640b9165293997e655f67f82c Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Wed, 13 Nov 2019 15:33:03 +0000 Subject: [PATCH 02/59] started work, committing quickly, laptop seems to be dying soon --- esmvalcore/_main.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/esmvalcore/_main.py b/esmvalcore/_main.py index a2994e7d01..e5c2674b5c 100755 --- a/esmvalcore/_main.py +++ b/esmvalcore/_main.py @@ -98,6 +98,15 @@ def get_args(): '--diagnostics', nargs='*', help="Only run the named diagnostics from the recipe.") + parser.add_argument( + '--cmor-checks', + type=str, + action='store_true', + help="Metadata CMOR checks strictness; \ + Optional: true; possible values: \ + none: no checks performed \ + relaxed: checks are performed but no errors raised; \ + default: full checks and errors raised.") args = parser.parse_args() return args @@ -143,6 +152,7 @@ def main(args): pattern if TASKSEP in pattern else pattern + TASKSEP + '*' for pattern in args.diagnostics or () } + cfg['cmor_checks_type'] = args.cmor_checks cfg['synda_download'] = args.synda_download for limit in ('max_datasets', 'max_years'): value = getattr(args, limit) From 861a91c9d6378658e7b7810c0025c4be81bce878 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Wed, 13 Nov 2019 15:50:12 +0000 Subject: [PATCH 03/59] talk to the Check --- esmvalcore/_recipe.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/esmvalcore/_recipe.py b/esmvalcore/_recipe.py index c75af61145..47173b7301 100644 --- a/esmvalcore/_recipe.py +++ b/esmvalcore/_recipe.py @@ -332,11 +332,15 @@ def _get_default_settings(variable, config_user, derive=False): # Configure CMOR metadata check if variable.get('cmor_table'): + raise_exception = True + if config_user['cmor_checks_type'] in ['none', 'relaxed']: + raise_exception = False settings['cmor_check_metadata'] = { 'cmor_table': variable['cmor_table'], 'mip': variable['mip'], 'short_name': variable['short_name'], 'frequency': variable['frequency'], + 'raise_exception': raise_exception, } # Configure final CMOR data check if variable.get('cmor_table'): From b5b96b19307f3e3b5e08439fd3820fc5b0e662e5 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Wed, 13 Nov 2019 16:51:30 +0000 Subject: [PATCH 04/59] communication with the cmor checker --- esmvalcore/_recipe.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/esmvalcore/_recipe.py b/esmvalcore/_recipe.py index 47173b7301..e0806a8b8a 100644 --- a/esmvalcore/_recipe.py +++ b/esmvalcore/_recipe.py @@ -333,14 +333,18 @@ def _get_default_settings(variable, config_user, derive=False): # Configure CMOR metadata check if variable.get('cmor_table'): raise_exception = True + report_only_warning = False if config_user['cmor_checks_type'] in ['none', 'relaxed']: raise_exception = False + if config_user['cmor_checks_type'] == 'relaxed': + report_only_warning = True settings['cmor_check_metadata'] = { 'cmor_table': variable['cmor_table'], 'mip': variable['mip'], 'short_name': variable['short_name'], 'frequency': variable['frequency'], 'raise_exception': raise_exception, + 'report_only_warning': report_only_warning, } # Configure final CMOR data check if variable.get('cmor_table'): From 9a6b26445cab57be422577cda541585c1519d2d1 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Wed, 13 Nov 2019 16:51:51 +0000 Subject: [PATCH 05/59] implemented tiered errors --- esmvalcore/cmor/check.py | 52 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index b0323e5274..c203fae053 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -58,12 +58,15 @@ def __init__(self, frequency=None, fail_on_error=False, raise_exception=True, + report_only_warning=False, automatic_fixes=False): self._cube = cube self._failerr = fail_on_error self._raise_exception = raise_exception + self._report_only_warning = report_only_warning self._errors = list() + self._strict_errors = list() self._warnings = list() self._debug_messages = list() self._cmor_var = var_info @@ -129,8 +132,18 @@ def report_errors(self, logger): msg = msg.format(self._cube.var_name, '\n '.join(self._errors), self._cube) if self._raise_exception: + logger.error(msg) + raise CMORCheckError(msg) + if self._report_only_warning: + logger.warning(msg) + if self.has_strict_errors(): + msg = 'There were strict errors in variable {}:\n{}\nin cube:\n{}' + msg = msg.format( + self._cube.var_name, '\n '.join(self._strict_errors), + self._cube) + if self._raise_exception: + logger.error(msg) raise CMORCheckError(msg) - logger.error(msg) def report_warnings(self, logger): """Report detected warnings to the given logger. @@ -583,6 +596,17 @@ def has_errors(self): """ return len(self._errors) > 0 + def has_strict_errors(self): + """Check if there are reported errors of strict nature. + + Returns + ------- + bool: + True if there are pending errors, False otherwise. + + """ + return len(self._strict_errors) > 0 + def has_warnings(self): """Check if there are reported warnings. @@ -624,6 +648,25 @@ def report_error(self, message, *args): raise CMORCheckError(msg + '\nin cube:\n{}'.format(self._cube)) self._errors.append(msg) + def report_strict_error(self, message, *args): + """Report a STRICT error. + + If fail_on_error is set to True, raises automatically. + If fail_on_error is set to False, stores it for later reports. + + Parameters + ---------- + message: str: unicode + Message for the error. + *args: + arguments to format the message string. + + """ + msg = message.format(*args) + if self._failerr: + raise CMORCheckError(msg + '\nin cube:\n{}'.format(self._cube)) + self._strict_errors.append(msg) + def report_warning(self, message, *args): """Report a warning. @@ -696,13 +739,15 @@ def _checker(cube): frequency=frequency, fail_on_error=fail_on_error, raise_exception=raise_exception, + report_only_warning=report_only_warning, automatic_fixes=automatic_fixes) return _checker def cmor_check_metadata(cube, cmor_table, mip, - short_name, frequency, raise_exception): + short_name, frequency, + raise_exception, report_only_warning): """Check if metadata conforms to variable's CMOR definiton. None of the checks at this step will force the cube to load the data. @@ -726,7 +771,8 @@ def cmor_check_metadata(cube, cmor_table, mip, """ checker = _get_cmor_checker(cmor_table, mip, short_name, frequency, - raise_exception=raise_exception) + raise_exception=raise_exception, + report_only_warning=report_only_warning) checker(cube).check_metadata() return cube From a5b768044f0644b72fcae1a935f4391d1e47c6db Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Wed, 13 Nov 2019 16:59:12 +0000 Subject: [PATCH 06/59] rephrased a bit the error messages --- esmvalcore/cmor/check.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index c203fae053..df78db44a5 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -128,7 +128,7 @@ def report_errors(self, logger): """ if self.has_errors(): - msg = 'There were errors in variable {}:\n{}\nin cube:\n{}' + msg = 'There are REGULAR CMOR errors in var {}:\n{}\nin cube:\n{}' msg = msg.format(self._cube.var_name, '\n '.join(self._errors), self._cube) if self._raise_exception: @@ -137,13 +137,13 @@ def report_errors(self, logger): if self._report_only_warning: logger.warning(msg) if self.has_strict_errors(): - msg = 'There were strict errors in variable {}:\n{}\nin cube:\n{}' + msg = 'There are STRICT CMOR errors in var {}:\n{}\nin cube:\n{}' msg = msg.format( self._cube.var_name, '\n '.join(self._strict_errors), self._cube) if self._raise_exception: - logger.error(msg) raise CMORCheckError(msg) + logger.error(msg) def report_warnings(self, logger): """Report detected warnings to the given logger. From 15919a7e7b2f24b605eb3e492d35d8045b4f48c8 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Fri, 15 Nov 2019 11:12:38 +0000 Subject: [PATCH 07/59] Bouwe suggestion Co-Authored-By: Bouwe Andela --- esmvalcore/_main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvalcore/_main.py b/esmvalcore/_main.py index e5c2674b5c..34aa2fc865 100755 --- a/esmvalcore/_main.py +++ b/esmvalcore/_main.py @@ -101,7 +101,7 @@ def get_args(): parser.add_argument( '--cmor-checks', type=str, - action='store_true', + choices=('none', 'relaxed', 'default'), help="Metadata CMOR checks strictness; \ Optional: true; possible values: \ none: no checks performed \ From edd1f943f4d8fab9e1197b8344a23a0951106230 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Fri, 15 Nov 2019 11:13:08 +0000 Subject: [PATCH 08/59] Bouwe suggestion Co-Authored-By: Bouwe Andela --- esmvalcore/_main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvalcore/_main.py b/esmvalcore/_main.py index 34aa2fc865..f6a5be3e6b 100755 --- a/esmvalcore/_main.py +++ b/esmvalcore/_main.py @@ -152,7 +152,7 @@ def main(args): pattern if TASKSEP in pattern else pattern + TASKSEP + '*' for pattern in args.diagnostics or () } - cfg['cmor_checks_type'] = args.cmor_checks + cfg['cmor_checks'] = args.cmor_checks cfg['synda_download'] = args.synda_download for limit in ('max_datasets', 'max_years'): value = getattr(args, limit) From 0cabd664fe4045bcf006477fc7d977843a9afc2a Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Fri, 15 Nov 2019 11:17:47 +0000 Subject: [PATCH 09/59] replaced config key --- esmvalcore/_recipe.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esmvalcore/_recipe.py b/esmvalcore/_recipe.py index e0806a8b8a..9592706e03 100644 --- a/esmvalcore/_recipe.py +++ b/esmvalcore/_recipe.py @@ -334,9 +334,9 @@ def _get_default_settings(variable, config_user, derive=False): if variable.get('cmor_table'): raise_exception = True report_only_warning = False - if config_user['cmor_checks_type'] in ['none', 'relaxed']: + if config_user['cmor_checks'] in ['none', 'relaxed']: raise_exception = False - if config_user['cmor_checks_type'] == 'relaxed': + if config_user['cmor_checks'] == 'relaxed': report_only_warning = True settings['cmor_check_metadata'] = { 'cmor_table': variable['cmor_table'], From 1c4701cf786a1a1fdbe2b560ec12df9068a0f115 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Fri, 15 Nov 2019 11:19:33 +0000 Subject: [PATCH 10/59] added default opt to cmd line --- esmvalcore/_main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/esmvalcore/_main.py b/esmvalcore/_main.py index f6a5be3e6b..3f482d36e5 100755 --- a/esmvalcore/_main.py +++ b/esmvalcore/_main.py @@ -102,6 +102,7 @@ def get_args(): '--cmor-checks', type=str, choices=('none', 'relaxed', 'default'), + default='default', help="Metadata CMOR checks strictness; \ Optional: true; possible values: \ none: no checks performed \ From 30eee1d52ab638535b1176d950ef44b6e1b1bfdb Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Fri, 15 Nov 2019 12:21:04 +0000 Subject: [PATCH 11/59] fixed tests --- tests/integration/test_recipe.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/integration/test_recipe.py b/tests/integration/test_recipe.py index 30b55c0c5e..bd40786c29 100644 --- a/tests/integration/test_recipe.py +++ b/tests/integration/test_recipe.py @@ -361,6 +361,8 @@ def test_default_preprocessor(tmp_path, patched_datafinder, config_user): 'mip': 'Oyr', 'short_name': 'chl', 'frequency': 'yr', + 'raise_exception': True, + 'report_only_warning': False, }, 'cmor_check_data': { 'cmor_table': 'CMIP5', @@ -442,6 +444,8 @@ def test_default_fx_preprocessor(tmp_path, patched_datafinder, config_user): 'mip': 'fx', 'short_name': 'sftlf', 'frequency': 'fx', + 'raise_exception': True, + 'report_only_warning': False, }, 'cmor_check_data': { 'cmor_table': 'CMIP5', From e71b6dcfa69242723be0193f40c8177d7d26d292 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Fri, 15 Nov 2019 12:21:21 +0000 Subject: [PATCH 12/59] fixed missing func arg --- esmvalcore/cmor/check.py | 1 + 1 file changed, 1 insertion(+) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index df78db44a5..cfe497335a 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -719,6 +719,7 @@ def _get_cmor_checker(table, frequency, fail_on_error=True, raise_exception=True, + report_only_warning=False, automatic_fixes=False): """Get a CMOR checker/fixer.""" if table not in CMOR_TABLES: From 694411b124895f026279345da7f127586cd22fd5 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Fri, 15 Nov 2019 12:21:37 +0000 Subject: [PATCH 13/59] fixed opt retrieval --- esmvalcore/_recipe.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/esmvalcore/_recipe.py b/esmvalcore/_recipe.py index 9592706e03..2a84325c37 100644 --- a/esmvalcore/_recipe.py +++ b/esmvalcore/_recipe.py @@ -334,9 +334,10 @@ def _get_default_settings(variable, config_user, derive=False): if variable.get('cmor_table'): raise_exception = True report_only_warning = False - if config_user['cmor_checks'] in ['none', 'relaxed']: + cmor_check = config_user.get('cmor_checks', None) + if cmor_check in ['none', 'relaxed']: raise_exception = False - if config_user['cmor_checks'] == 'relaxed': + if cmor_check == 'relaxed': report_only_warning = True settings['cmor_check_metadata'] = { 'cmor_table': variable['cmor_table'], From b1dfb5140edc5c90d534faf13f7a67a9c541e9f6 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Fri, 15 Nov 2019 16:11:14 +0100 Subject: [PATCH 14/59] Do not report anything in fix_metadata step --- esmvalcore/cmor/fix.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/esmvalcore/cmor/fix.py b/esmvalcore/cmor/fix.py index 2881d49aa7..f16252ba08 100644 --- a/esmvalcore/cmor/fix.py +++ b/esmvalcore/cmor/fix.py @@ -141,6 +141,8 @@ def fix_metadata(cubes, mip=mip, short_name=short_name, fail_on_error=False, + raise_exception=False, + report_only_warning=False, automatic_fixes=True) cube = checker(cube).check_metadata() cube.attributes.pop('source_file', None) From bb7037314f307e3f4da40829d3b104514f755830 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Fri, 15 Nov 2019 16:11:53 +0100 Subject: [PATCH 15/59] Relax some check_metadata checks --- esmvalcore/cmor/check.py | 87 ++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 52 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index cfe497335a..39f3f90e89 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -66,7 +66,6 @@ def __init__(self, self._raise_exception = raise_exception self._report_only_warning = report_only_warning self._errors = list() - self._strict_errors = list() self._warnings = list() self._debug_messages = list() self._cmor_var = var_info @@ -109,7 +108,7 @@ def check_metadata(self, logger=None): self.report_debug_messages(logger) self.report_warnings(logger) - self.report_errors(logger) + self.report_errors() if self.frequency != 'fx': self._add_auxiliar_time_coordinates() @@ -118,7 +117,7 @@ def check_metadata(self, logger=None): else: return self._cube - def report_errors(self, logger): + def report_errors(self): """Report detected errors. Raises @@ -128,22 +127,10 @@ def report_errors(self, logger): """ if self.has_errors(): - msg = 'There are REGULAR CMOR errors in var {}:\n{}\nin cube:\n{}' + msg = 'There were errors in variable {}:\n{}\nin cube:\n{}' msg = msg.format(self._cube.var_name, '\n '.join(self._errors), self._cube) - if self._raise_exception: - logger.error(msg) - raise CMORCheckError(msg) - if self._report_only_warning: - logger.warning(msg) - if self.has_strict_errors(): - msg = 'There are STRICT CMOR errors in var {}:\n{}\nin cube:\n{}' - msg = msg.format( - self._cube.var_name, '\n '.join(self._strict_errors), - self._cube) - if self._raise_exception: - raise CMORCheckError(msg) - logger.error(msg) + raise CMORCheckError(msg) def report_warnings(self, logger): """Report detected warnings to the given logger. @@ -199,7 +186,7 @@ def check_data(self, logger=None): self._check_coords_data() self.report_warnings(logger) - self.report_errors(logger) + self.report_errors() return self._cube def _check_fill_value(self): @@ -224,7 +211,7 @@ def _check_var_metadata(self): ) self._cube.standard_name = self._cmor_var.standard_name else: - self.report_error( + self.report_strict_error( self._attr_msg, self._cube.var_name, 'standard_name', self._cmor_var.standard_name, self._cube.standard_name ) @@ -240,7 +227,7 @@ def _check_var_metadata(self): ) self._cube.long_name = self._cmor_var.long_name else: - self.report_error( + self.report_strict_error( self._attr_msg, self._cube.var_name, 'long_name', self._cmor_var.long_name, self._cube.long_name ) @@ -255,9 +242,9 @@ def _check_var_metadata(self): units = self._get_effective_units() if not self._cube.units.is_convertible(units): - self.report_error(f'Variable {self._cube.var_name} units ' - f'{self._cube.units} can not be ' - f'converted to {self._cmor_var.units}') + self.report_strict_error(f'Variable {self._cube.var_name} units ' + f'{self._cube.units} can not be ' + f'converted to {self._cmor_var.units}') # Check other variable attributes that match entries in cube.attributes attrs = ('positive', ) @@ -268,9 +255,9 @@ def _check_var_metadata(self): self.report_warning('{}: attribute {} not present', self._cube.var_name, attr) elif self._cube.attributes[attr] != attr_value: - self.report_error(self._attr_msg, self._cube.var_name, - attr, attr_value, - self._cube.attributes[attr]) + self.report_strict_error(self._attr_msg, self._cube.var_name, + attr, attr_value, + self._cube.attributes[attr]) def _get_effective_units(self): """Get effective units.""" @@ -334,9 +321,8 @@ def _check_dim_names(self): ) coord.var_name = coordinate.out_name else: - self.report_error( + self.report_strict_error( 'Coordinate {0} has var name {1}' - 'Coordinate {0} has var name {1} ' 'instead of {2}', coordinate.name, coord.var_name, @@ -480,7 +466,7 @@ def _check_time_coord(self): var_name = coord.var_name if not coord.is_monotonic(): - self.report_error( + self.report_strict_error( 'Time coordinate for var {} is not monotonic', var_name ) @@ -541,7 +527,7 @@ def _check_time_coord(self): if second_month != second.month or \ second_year != second.year: msg = '{}: Frequency {} does not match input data' - self.report_error(msg, var_name, freq) + self.report_strict_error(msg, var_name, freq) break elif freq == 'yr': for i in range(len(coord.points) - 1): @@ -550,7 +536,7 @@ def _check_time_coord(self): second_month = first.month + 1 if first.year + 1 != second.year: msg = '{}: Frequency {} does not match input data' - self.report_error(msg, var_name, freq) + self.report_strict_error(msg, var_name, freq) break else: if freq in intervals: @@ -566,14 +552,14 @@ def _check_time_coord(self): target_interval = (frequency - tol, frequency + tol) else: msg = '{}: Frequency {} not supported by checker' - self.report_error(msg, var_name, freq) + self.report_strict_error(msg, var_name, freq) return for i in range(len(coord.points) - 1): interval = coord.points[i + 1] - coord.points[i] if (interval < target_interval[0] or interval > target_interval[1]): msg = '{}: Frequency {} does not match input data' - self.report_error(msg, var_name, freq) + self.report_strict_error(msg, var_name, freq) break @staticmethod @@ -596,17 +582,6 @@ def has_errors(self): """ return len(self._errors) > 0 - def has_strict_errors(self): - """Check if there are reported errors of strict nature. - - Returns - ------- - bool: - True if there are pending errors, False otherwise. - - """ - return len(self._strict_errors) > 0 - def has_warnings(self): """Check if there are reported warnings. @@ -650,9 +625,10 @@ def report_error(self, message, *args): def report_strict_error(self, message, *args): """Report a STRICT error. - - If fail_on_error is set to True, raises automatically. - If fail_on_error is set to False, stores it for later reports. + If raise_exception is set to True and fail_on_error is set to True, raises automatically. + If raise_exception is set to True and fail_on_error is set to False, stores it for later reports. + If raise_exception is set to False and report_only_warning is set to True, go to report_warning. + If raise_exception is set to False and report_only_warning is set to False, do not report. Parameters ---------- @@ -663,9 +639,12 @@ def report_strict_error(self, message, *args): """ msg = message.format(*args) - if self._failerr: - raise CMORCheckError(msg + '\nin cube:\n{}'.format(self._cube)) - self._strict_errors.append(msg) + if self._raise_exception: + if self._failerr: + raise CMORCheckError(msg + '\nin cube:\n{}'.format(self._cube)) + self._errors.append(msg) + if self._report_only_warning: + self.report_warning(message, *args) def report_warning(self, message, *args): """Report a warning. @@ -802,7 +781,9 @@ def cmor_check_data(cube, cmor_table, mip, short_name, frequency): return cube -def cmor_check(cube, cmor_table, mip, short_name, frequency): +def cmor_check(cube, cmor_table, mip, short_name, frequency, + raise_exception, + report_only_warning): """Check if cube conforms to variable's CMOR definiton. Equivalent to calling cmor_check_metadata and cmor_check_data @@ -822,6 +803,8 @@ def cmor_check(cube, cmor_table, mip, short_name, frequency): Data frequency. """ - cmor_check_metadata(cube, cmor_table, mip, short_name, frequency) + cmor_check_metadata(cube, cmor_table, mip, short_name, frequency, + raise_exception=raise_exception, + report_only_warning=report_only_warning) cmor_check_data(cube, cmor_table, mip, short_name, frequency) return cube From 3dee4ae042a6724d0e629f2a8b420a3a9de4b06c Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Mon, 18 Nov 2019 16:06:14 +0000 Subject: [PATCH 16/59] putting command line args dependent options in fix --- esmvalcore/_recipe.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/esmvalcore/_recipe.py b/esmvalcore/_recipe.py index 2a84325c37..a6d6eae687 100644 --- a/esmvalcore/_recipe.py +++ b/esmvalcore/_recipe.py @@ -309,6 +309,15 @@ def _get_default_settings(variable, config_user, derive=False): fix['cmor_table'] = variable['cmor_table'] fix['mip'] = variable['mip'] fix['frequency'] = variable['frequency'] + cmor_check = config_user.get('cmor_checks', None) + raise_exception = True + report_only_warning = False + if cmor_check in ['none', 'relaxed']: + raise_exception = False + if cmor_check == 'relaxed': + report_only_warning = True + fix['raise_exception'] = raise_exception + fix['report_only_warning'] = report_only_warning settings['fix_data'] = dict(fix) settings['fix_metadata'] = dict(fix) @@ -339,6 +348,7 @@ def _get_default_settings(variable, config_user, derive=False): raise_exception = False if cmor_check == 'relaxed': report_only_warning = True + print("RAISERECIPE", raise_exception) settings['cmor_check_metadata'] = { 'cmor_table': variable['cmor_table'], 'mip': variable['mip'], From c7d75a364ecaa1165342236340d05fc38f4d7977 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Mon, 18 Nov 2019 16:07:09 +0000 Subject: [PATCH 17/59] making the implementation actually work --- esmvalcore/cmor/check.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 39f3f90e89..3cdb183c84 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -112,10 +112,7 @@ def check_metadata(self, logger=None): if self.frequency != 'fx': self._add_auxiliar_time_coordinates() - if self.has_errors(): - return None - else: - return self._cube + return self._cube def report_errors(self): """Report detected errors. @@ -130,7 +127,8 @@ def report_errors(self): msg = 'There were errors in variable {}:\n{}\nin cube:\n{}' msg = msg.format(self._cube.var_name, '\n '.join(self._errors), self._cube) - raise CMORCheckError(msg) + if self._raise_exception: + raise CMORCheckError(msg) def report_warnings(self, logger): """Report detected warnings to the given logger. @@ -696,7 +694,7 @@ def _get_cmor_checker(table, mip, short_name, frequency, - fail_on_error=True, + fail_on_error=False, raise_exception=True, report_only_warning=False, automatic_fixes=False): From 847e8f1d2807c15efb17f27e14c728fbec8bff5b Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Mon, 18 Nov 2019 16:07:50 +0000 Subject: [PATCH 18/59] adding func args --- esmvalcore/cmor/fix.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/esmvalcore/cmor/fix.py b/esmvalcore/cmor/fix.py index f16252ba08..047163442c 100644 --- a/esmvalcore/cmor/fix.py +++ b/esmvalcore/cmor/fix.py @@ -56,7 +56,9 @@ def fix_metadata(cubes, dataset, cmor_table=None, mip=None, - frequency=None): + frequency=None, + raise_exception=True, + report_only_warning=False, ): """ Fix cube metadata if fixes are required and check it anyway. @@ -140,9 +142,9 @@ def fix_metadata(cubes, table=cmor_table, mip=mip, short_name=short_name, + raise_exception=raise_exception, + report_only_warning=report_only_warning, fail_on_error=False, - raise_exception=False, - report_only_warning=False, automatic_fixes=True) cube = checker(cube).check_metadata() cube.attributes.pop('source_file', None) @@ -156,7 +158,9 @@ def fix_data(cube, dataset, cmor_table=None, mip=None, - frequency=None): + frequency=None, + raise_exception=True, + report_only_warning=False, ): """ Fix cube data if fixes add present and check it anyway. From c7971a56aa79014b49d88c609bfb192dc8144066 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Mon, 18 Nov 2019 16:39:38 +0000 Subject: [PATCH 19/59] fixed lines too long --- esmvalcore/cmor/check.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 3cdb183c84..1ce573bb55 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -240,9 +240,10 @@ def _check_var_metadata(self): units = self._get_effective_units() if not self._cube.units.is_convertible(units): - self.report_strict_error(f'Variable {self._cube.var_name} units ' - f'{self._cube.units} can not be ' - f'converted to {self._cmor_var.units}') + self.report_strict_error( + f'Variable {self._cube.var_name} units ' + f'{self._cube.units} can not be ' + f'converted to {self._cmor_var.units}') # Check other variable attributes that match entries in cube.attributes attrs = ('positive', ) @@ -253,9 +254,10 @@ def _check_var_metadata(self): self.report_warning('{}: attribute {} not present', self._cube.var_name, attr) elif self._cube.attributes[attr] != attr_value: - self.report_strict_error(self._attr_msg, self._cube.var_name, - attr, attr_value, - self._cube.attributes[attr]) + self.report_strict_error( + self._attr_msg, self._cube.var_name, + attr, attr_value, + self._cube.attributes[attr]) def _get_effective_units(self): """Get effective units.""" @@ -623,10 +625,10 @@ def report_error(self, message, *args): def report_strict_error(self, message, *args): """Report a STRICT error. - If raise_exception is set to True and fail_on_error is set to True, raises automatically. - If raise_exception is set to True and fail_on_error is set to False, stores it for later reports. - If raise_exception is set to False and report_only_warning is set to True, go to report_warning. - If raise_exception is set to False and report_only_warning is set to False, do not report. + If raise_exception True and fail_on_error True, raises automatically. + If raise_exception True and fail_on_error False, stores it. + If raise_exception False and report_only_warning True report_warning. + If raise_exception False and report_only_warning False do not report. Parameters ---------- From 23e14de130287a5bc00e026bf8f4d64c1a76b5ac Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Mon, 18 Nov 2019 16:39:56 +0000 Subject: [PATCH 20/59] fixed recipe tests --- tests/integration/test_recipe.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/integration/test_recipe.py b/tests/integration/test_recipe.py index bd40786c29..5a44733d4f 100644 --- a/tests/integration/test_recipe.py +++ b/tests/integration/test_recipe.py @@ -339,6 +339,8 @@ def test_default_preprocessor(tmp_path, patched_datafinder, config_user): 'cmor_table': 'CMIP5', 'mip': 'Oyr', 'frequency': 'yr', + 'raise_exception': True, + 'report_only_warning': False, }, 'fix_metadata': { 'project': 'CMIP5', @@ -347,6 +349,8 @@ def test_default_preprocessor(tmp_path, patched_datafinder, config_user): 'cmor_table': 'CMIP5', 'mip': 'Oyr', 'frequency': 'yr', + 'raise_exception': True, + 'report_only_warning': False, }, 'extract_time': { 'start_year': 2000, @@ -430,6 +434,8 @@ def test_default_fx_preprocessor(tmp_path, patched_datafinder, config_user): 'cmor_table': 'CMIP5', 'mip': 'fx', 'frequency': 'fx', + 'raise_exception': True, + 'report_only_warning': False, }, 'fix_metadata': { 'project': 'CMIP5', @@ -438,6 +444,8 @@ def test_default_fx_preprocessor(tmp_path, patched_datafinder, config_user): 'cmor_table': 'CMIP5', 'mip': 'fx', 'frequency': 'fx', + 'raise_exception': True, + 'report_only_warning': False, }, 'cmor_check_metadata': { 'cmor_table': 'CMIP5', From 0f9479dc7e84e95ed48ea6b9fa8f31a1f706503e Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Mon, 18 Nov 2019 16:40:14 +0000 Subject: [PATCH 21/59] fixed call to fix test --- tests/unit/cmor/test_fix.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/cmor/test_fix.py b/tests/unit/cmor/test_fix.py index a449c822b2..48cfe8dda3 100644 --- a/tests/unit/cmor/test_fix.py +++ b/tests/unit/cmor/test_fix.py @@ -149,7 +149,9 @@ def test_cmor_checker_called(self): frequency='frequency', mip='mip', short_name='short_name', - table='cmor_table') + table='cmor_table', + raise_exception=True, + report_only_warning=False) checker.assert_called_once_with(self.cube) checker.return_value.check_metadata.assert_called_once_with() From f59418e29644e82dca2b88a2141e206fa6903148 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Mon, 18 Nov 2019 16:40:48 +0000 Subject: [PATCH 22/59] removed debug messgae --- esmvalcore/_recipe.py | 1 - 1 file changed, 1 deletion(-) diff --git a/esmvalcore/_recipe.py b/esmvalcore/_recipe.py index a6d6eae687..4298af34c3 100644 --- a/esmvalcore/_recipe.py +++ b/esmvalcore/_recipe.py @@ -348,7 +348,6 @@ def _get_default_settings(variable, config_user, derive=False): raise_exception = False if cmor_check == 'relaxed': report_only_warning = True - print("RAISERECIPE", raise_exception) settings['cmor_check_metadata'] = { 'cmor_table': variable['cmor_table'], 'mip': variable['mip'], From bd9b4c9807286b0b231ec99b5469ac2db1cf59a3 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Mon, 25 Nov 2019 12:11:50 +0100 Subject: [PATCH 23/59] Remove condition for regular errors --- esmvalcore/cmor/check.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 1ce573bb55..78a8e21a04 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -127,8 +127,7 @@ def report_errors(self): msg = 'There were errors in variable {}:\n{}\nin cube:\n{}' msg = msg.format(self._cube.var_name, '\n '.join(self._errors), self._cube) - if self._raise_exception: - raise CMORCheckError(msg) + raise CMORCheckError(msg) def report_warnings(self, logger): """Report detected warnings to the given logger. From 4f6b6a422d08b03e41190185c213c1b702527ce3 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Tue, 26 Nov 2019 09:51:05 +0100 Subject: [PATCH 24/59] Add unit tests --- tests/unit/cmor/test_cmor_check.py | 237 ++++++++++++++++++++++++++++- 1 file changed, 231 insertions(+), 6 deletions(-) diff --git a/tests/unit/cmor/test_cmor_check.py b/tests/unit/cmor/test_cmor_check.py index 9caa5beea8..6a967ca234 100644 --- a/tests/unit/cmor/test_cmor_check.py +++ b/tests/unit/cmor/test_cmor_check.py @@ -133,18 +133,36 @@ def test_check(self): """Test checks succeeds for a good cube.""" self._check_cube() - def _check_cube(self, automatic_fixes=False, frequency=None): + def _check_cube(self, automatic_fixes=False, frequency=None, + raise_exception=True, report_only_warning=False): """Apply checks and optionally automatic fixes to self.cube.""" def checker(cube): return CMORCheck( cube, self.var_info, automatic_fixes=automatic_fixes, - frequency=frequency) + frequency=frequency, + raise_exception=raise_exception, + report_only_warning=report_only_warning) self.cube = checker(self.cube).check_metadata() self.cube = checker(self.cube).check_data() + def _check_cube_metadata(self, automatic_fixes=False, frequency=None, + raise_exception=True, report_only_warning=False): + """Apply checks and optionally automatic fixes to self.cube.""" + def checker(cube): + return CMORCheck( + cube, + self.var_info, + automatic_fixes=automatic_fixes, + frequency=frequency, + raise_exception=raise_exception, + report_only_warning=report_only_warning) + + self.cube = checker(self.cube).check_metadata() + + def test_check_with_month_number(self): """Test checks succeeds for a good cube with month number.""" iris.coord_categorisation.add_month_number(self.cube, 'time') @@ -258,22 +276,229 @@ def test_rank_unestructured_grid(self): self.cube.add_aux_coord(new_lat, 1) self._check_cube() - def _check_fails_in_metadata(self, automatic_fixes=False, frequency=None): + def test_check_bad_var_standard_name_strict_flag(self): + """Test check fails for a bad variable standard_name with + --cmor-check strict.""" + self.cube = self.get_cube(self.var_info) + self.cube.standard_name = 'wind_speed' + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=True, + report_only_warning=False) + + def test_check_bad_var_long_name_strict_flag(self): + """Test check fails for a bad variable long_name with + --cmor-check strict.""" + self.cube = self.get_cube(self.var_info) + self.cube.long_name = "Near-Surface Wind Speed" + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=True, + report_only_warning=False) + + def test_check_bad_var_units_strict_flag(self): + """Test check fails for a bad variable units with + --cmor-check strict.""" + self.cube = self.get_cube(self.var_info) + self.cube.units = "kg" + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=True, + report_only_warning=False) + + def test_check_bad_attributes_strict_flag(self): + """Test check fails for a bad variable attribute with + --cmor-check strict.""" + self.var_info.standard_name = "surface_upward_latent_heat_flux" + self.var_info.positive = "up" + self.cube = self.get_cube(self.var_info) + self.cube.attributes['positive'] = "Wrong attribute" + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=True, + report_only_warning=False) + + def test_check_bad_rank_strict_flag(self): + """Test check fails for a bad variable rank with + --cmor-check strict.""" + lat = iris.coords.AuxCoord.from_coord(self.cube.coord('latitude')) + self.cube.remove_coord('latitude') + self.cube.add_aux_coord(lat, self.cube.coord_dims('longitude')) + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=True, + report_only_warning=False) + + def test_check_bad_coord_var_name_strict_flag(self): + """Test check fails for bad coord var_name with + --cmor-check strict""" + self.var_info.table_type = 'CMIP5' + self.cube.coord('longitude').var_name = 'bad_name' + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=True, + report_only_warning=False) + + def test_check_missing_cord_strict_flag(self): + """Test check fails for missing coord with --cmor-check strict""" + self.var_info.table_type = 'CMIP5' + self.cube.remove_coord('longitude') + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=True, + report_only_warning=False) + + def test_check_bad_var_standard_name_relaxed_flag(self): + """Test check reports warning for a bad variable standard_name with + --cmor-check relaxed.""" + self.cube = self.get_cube(self.var_info) + self.cube.standard_name = 'wind_speed' + self._check_warnings_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=True) + + def test_check_bad_var_long_name_relaxed_flag(self): + """Test check reports warning for a bad variable long_name with + --cmor-check relaxed.""" + self.cube = self.get_cube(self.var_info) + self.cube.long_name = "Near-Surface Wind Speed" + self._check_warnings_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=True) + + def test_check_bad_var_units_relaxed_flag(self): + """Test check reports warning for a bad variable units with + --cmor-check relaxed.""" + self.cube = self.get_cube(self.var_info) + self.cube.units = "kg" + self._check_warnings_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=True) + + def test_check_bad_attributes_relaxed_flag(self): + """Test check report warnings for a bad variable attribute with + --cmor-check relaxed.""" + self.var_info.standard_name = "surface_upward_latent_heat_flux" + self.var_info.positive = "up" + self.cube = self.get_cube(self.var_info) + self.cube.attributes['positive'] = "Wrong attribute" + self._check_warnings_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=True) + + def test_check_bad_rank_relaxed_flag(self): + """Test check report warnings for a bad variable rank with + --cmor-check relaxed.""" + lat = iris.coords.AuxCoord.from_coord(self.cube.coord('latitude')) + self.cube.remove_coord('latitude') + self.cube.add_aux_coord(lat, self.cube.coord_dims('longitude')) + self._check_warnings_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=True) + + def test_check_bad_coord_standard_name_relaxed_flag(self): + """Test check reports warning for bad coord var_name with + --cmor-check relaxed""" + self.var_info.table_type = 'CMIP5' + self.cube.coord('longitude').var_name = 'bad_name' + self._check_warnings_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=True) + + def test_check_missing_coord_relaxed_flag(self): + """Test check fails for missing coord with --cmor-check relaxed""" + self.var_info.table_type = 'CMIP5' + self.cube.remove_coord('longitude') + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=True) + + def test_check_bad_var_standard_name_none_flag(self): + """Test check does not report for a bad variable standard_name with + --cmor-check none.""" + self.cube = self.get_cube(self.var_info) + self.cube.standard_name = 'wind_speed' + self._check_no_reports_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=False) + + def test_check_bad_var_long_name_none_flag(self): + """Test check does not report for a bad variable long_name with + --cmor-check none.""" + self.cube = self.get_cube(self.var_info) + self.cube.long_name = "Near-Surface Wind Speed" + self._check_no_reports_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=False) + + def test_check_bad_var_units_none_flag(self): + """Test check does not report for a bad variable unit with + --cmor-check none.""" + self.cube = self.get_cube(self.var_info) + self.cube.units = "kg" + self._check_no_reports_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=False) + + def test_check_bad_attributes_none_flag(self): + """Test check does not report for a bad variable attribute with + --cmor-check none.""" + self.var_info.standard_name = "surface_upward_latent_heat_flux" + self.var_info.positive = "up" + self.cube = self.get_cube(self.var_info) + self.cube.attributes['positive'] = "Wrong attribute" + self._check_no_reports_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=False) + + def test_check_bad_rank_none_flag(self): + """Test check does not report for a bad variable rank with + --cmor-check relaxed.""" + lat = iris.coords.AuxCoord.from_coord(self.cube.coord('latitude')) + self.cube.remove_coord('latitude') + self.cube.add_aux_coord(lat, self.cube.coord_dims('longitude')) + self._check_no_reports_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=False) + + def test_check_bad_coord_standard_name_none_flag(self): + """Test check reports nothing for bad coord var_name with + --cmor-check none.""" + self.var_info.table_type = 'CMIP5' + self.cube.coord('longitude').var_name = 'bad_name' + self._check_no_reports_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=False) + + def _check_fails_in_metadata(self, automatic_fixes=False, frequency=None, + raise_exception=True, + report_only_warning=False): checker = CMORCheck( self.cube, self.var_info, automatic_fixes=automatic_fixes, - frequency=frequency) + frequency=frequency, + raise_exception=raise_exception, + report_only_warning=report_only_warning) with self.assertRaises(CMORCheckError): checker.check_metadata() - def _check_warnings_on_metadata(self, automatic_fixes=False): + def _check_warnings_on_metadata(self, automatic_fixes=False, + raise_exception=True, + report_only_warning=False): checker = CMORCheck( - self.cube, self.var_info, automatic_fixes=automatic_fixes + self.cube, self.var_info, automatic_fixes=automatic_fixes, + raise_exception=raise_exception, + report_only_warning=report_only_warning ) checker.check_metadata() self.assertTrue(checker.has_warnings()) + def _check_no_reports_on_metadata(self, automatic_fixes=False, + raise_exception=False, + report_only_warning=False): + checker = CMORCheck( + self.cube, self.var_info, automatic_fixes=automatic_fixes, + raise_exception=raise_exception, + report_only_warning=report_only_warning + ) + checker.check_metadata() + self.assertFalse(checker.has_errors()) + self.assertFalse(checker.has_warnings()) + def _check_debug_messages_on_metadata(self): checker = CMORCheck(self.cube, self.var_info) checker.check_metadata() From 06410b261c23da5cce17abfbc2f43ddc7b720801 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Fri, 29 Nov 2019 15:47:05 +0000 Subject: [PATCH 25/59] fixed linter --- tests/unit/cmor/test_cmor_check.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/unit/cmor/test_cmor_check.py b/tests/unit/cmor/test_cmor_check.py index 6a967ca234..11ff86a24e 100644 --- a/tests/unit/cmor/test_cmor_check.py +++ b/tests/unit/cmor/test_cmor_check.py @@ -162,7 +162,6 @@ def checker(cube): self.cube = checker(self.cube).check_metadata() - def test_check_with_month_number(self): """Test checks succeeds for a good cube with month number.""" iris.coord_categorisation.add_month_number(self.cube, 'time') @@ -347,8 +346,8 @@ def test_check_bad_var_standard_name_relaxed_flag(self): self.cube = self.get_cube(self.var_info) self.cube.standard_name = 'wind_speed' self._check_warnings_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=True) + raise_exception=False, + report_only_warning=True) def test_check_bad_var_long_name_relaxed_flag(self): """Test check reports warning for a bad variable long_name with @@ -488,8 +487,8 @@ def _check_warnings_on_metadata(self, automatic_fixes=False, self.assertTrue(checker.has_warnings()) def _check_no_reports_on_metadata(self, automatic_fixes=False, - raise_exception=False, - report_only_warning=False): + raise_exception=False, + report_only_warning=False): checker = CMORCheck( self.cube, self.var_info, automatic_fixes=automatic_fixes, raise_exception=raise_exception, From 57dd77bbd6128f058a3f9c2451ab900a970c7944 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Fri, 29 Nov 2019 16:01:30 +0000 Subject: [PATCH 26/59] fixed rank check --- esmvalcore/cmor/check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 78a8e21a04..9909298f22 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -284,8 +284,8 @@ def _check_rank(self): # Check number of dimension coords matches rank if self._cube.ndim != rank: - self.report_error(self._does_msg, self._cube.var_name, - 'match coordinate rank') + self.report_strict_error(self._does_msg, self._cube.var_name, + 'match coordinate rank') def _check_dim_names(self): """Check dimension names.""" From 3e27e49eb675f2fa55c604c505d3405fa90b0d74 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Mon, 2 Dec 2019 15:37:22 +0100 Subject: [PATCH 27/59] Relax check on presence of some coordinates --- esmvalcore/cmor/check.py | 10 +++- tests/unit/cmor/test_cmor_check.py | 95 ++++++++++++++++++++++++++++-- 2 files changed, 99 insertions(+), 6 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 9909298f22..b7ce80b440 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -328,8 +328,14 @@ def _check_dim_names(self): coordinate.out_name, ) except iris.exceptions.CoordinateNotFoundError: - self.report_error(self._does_msg, coordinate.name, - 'exist') + if coordinate.standard_name in ['time', 'latitude', + 'longitude']: + self.report_error(self._does_msg, coordinate.name, + 'exist') + else: + self.report_strict_error(self._does_msg, + coordinate.name, + 'exist') def _check_coords(self): """Check coordinates.""" diff --git a/tests/unit/cmor/test_cmor_check.py b/tests/unit/cmor/test_cmor_check.py index 11ff86a24e..7bcdebc98d 100644 --- a/tests/unit/cmor/test_cmor_check.py +++ b/tests/unit/cmor/test_cmor_check.py @@ -332,14 +332,40 @@ def test_check_bad_coord_var_name_strict_flag(self): raise_exception=True, report_only_warning=False) - def test_check_missing_cord_strict_flag(self): - """Test check fails for missing coord with --cmor-check strict""" + def test_check_missing_lon_strict_flag(self): + """Test check fails for missing longitude with --cmor-check strict""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('longitude') self._check_fails_in_metadata(automatic_fixes=False, raise_exception=True, report_only_warning=False) + def test_check_missing_lat_strict_flag(self): + """Test check fails for missing latitude with --cmor-check strict""" + self.var_info.table_type = 'CMIP5' + self.cube.remove_coord('latitude') + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=True, + report_only_warning=False) + + def test_check_missing_time_strict_flag(self): + """Test check fails for missing time with --cmor-check strict""" + self.var_info.table_type = 'CMIP5' + self.cube.remove_coord('time') + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=True, + report_only_warning=False) + + def test_check_missing_coord_strict_flag(self): + """Test check fails for missing coord other than lat and lon + with --cmor-check strict""" + self.cube = self.get_cube(self.var_info) + self.var_info.coordinates = {'height2m': CoordinateInfoMock('height2m') + } + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=True, + report_only_warning=False) + def test_check_bad_var_standard_name_relaxed_flag(self): """Test check reports warning for a bad variable standard_name with --cmor-check relaxed.""" @@ -397,14 +423,40 @@ def test_check_bad_coord_standard_name_relaxed_flag(self): raise_exception=False, report_only_warning=True) - def test_check_missing_coord_relaxed_flag(self): - """Test check fails for missing coord with --cmor-check relaxed""" + def test_check_missing_lon_relaxed_flag(self): + """Test check fails for missing longitude with --cmor-check relaxed""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('longitude') self._check_fails_in_metadata(automatic_fixes=False, raise_exception=False, report_only_warning=True) + def test_check_missing_lat_relaxed_flag(self): + """Test check fails for missing latitude with --cmor-check relaxed""" + self.var_info.table_type = 'CMIP5' + self.cube.remove_coord('latitude') + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=True) + + def test_check_missing_time_relaxed_flag(self): + """Test check fails for missing latitude with --cmor-check relaxed""" + self.var_info.table_type = 'CMIP5' + self.cube.remove_coord('time') + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=True) + + def test_check_missing_coord_relaxed_flag(self): + """Test check reports warning for missing coord other than lat and lon + with --cmor-check strict""" + self.cube = self.get_cube(self.var_info) + self.var_info.coordinates = {'height2m': CoordinateInfoMock('height2m') + } + self._check_warnings_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=True) + def test_check_bad_var_standard_name_none_flag(self): """Test check does not report for a bad variable standard_name with --cmor-check none.""" @@ -462,6 +514,41 @@ def test_check_bad_coord_standard_name_none_flag(self): raise_exception=False, report_only_warning=False) + def test_check_missing_lon_none_flag(self): + """Test check fails for missing longitude with --cmor-check none""" + self.var_info.table_type = 'CMIP5' + self.cube.remove_coord('longitude') + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=False) + + def test_check_missing_lat_none_flag(self): + """Test check fails for missing latitude with --cmor-check none""" + self.var_info.table_type = 'CMIP5' + self.cube.remove_coord('latitude') + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=False) + + def test_check_missing_time_none_flag(self): + """Test check fails for missing time with --cmor-check none""" + self.var_info.table_type = 'CMIP5' + self.cube.remove_coord('time') + self._check_fails_in_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=False) + + def test_check_missing_coord_none_flag(self): + """Test check reports nothing for missing coord other than lat, lon and + time with --cmor-check none""" + self.cube = self.get_cube(self.var_info) + self.var_info.coordinates = {'height2m': CoordinateInfoMock('height2m') + } + self._check_no_reports_on_metadata(automatic_fixes=False, + raise_exception=False, + report_only_warning=False) + + def _check_fails_in_metadata(self, automatic_fixes=False, frequency=None, raise_exception=True, report_only_warning=False): From 8f2e8dc44df30a49656398f46c0b07944e7db482 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Mon, 2 Dec 2019 15:55:14 +0100 Subject: [PATCH 28/59] Fix style issues --- esmvalcore/cmor/check.py | 8 ++------ tests/unit/cmor/test_cmor_check.py | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index b7ce80b440..e72d9b6f9b 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -630,8 +630,7 @@ def report_error(self, message, *args): def report_strict_error(self, message, *args): """Report a STRICT error. - If raise_exception True and fail_on_error True, raises automatically. - If raise_exception True and fail_on_error False, stores it. + If raise_exception True report error If raise_exception False and report_only_warning True report_warning. If raise_exception False and report_only_warning False do not report. @@ -643,11 +642,8 @@ def report_strict_error(self, message, *args): arguments to format the message string. """ - msg = message.format(*args) if self._raise_exception: - if self._failerr: - raise CMORCheckError(msg + '\nin cube:\n{}'.format(self._cube)) - self._errors.append(msg) + self.report_error(message, *args) if self._report_only_warning: self.report_warning(message, *args) diff --git a/tests/unit/cmor/test_cmor_check.py b/tests/unit/cmor/test_cmor_check.py index 7bcdebc98d..964f14aee0 100644 --- a/tests/unit/cmor/test_cmor_check.py +++ b/tests/unit/cmor/test_cmor_check.py @@ -449,7 +449,7 @@ def test_check_missing_time_relaxed_flag(self): def test_check_missing_coord_relaxed_flag(self): """Test check reports warning for missing coord other than lat and lon - with --cmor-check strict""" + with --cmor-check relaxed""" self.cube = self.get_cube(self.var_info) self.var_info.coordinates = {'height2m': CoordinateInfoMock('height2m') } From 84ded5a5fad7d5fbfd2145ce0e82f4608c9e8383 Mon Sep 17 00:00:00 2001 From: Javier Vegas-Regidor Date: Tue, 3 Dec 2019 11:30:36 +0100 Subject: [PATCH 29/59] Add support for diferent error levels in checker --- esmvalcore/_main.py | 26 ++-- esmvalcore/_recipe.py | 21 +-- esmvalcore/cmor/check.py | 229 +++++++++++++++-------------- esmvalcore/cmor/fix.py | 11 +- tests/integration/test_recipe.py | 21 ++- tests/unit/cmor/test_cmor_check.py | 163 +++++++------------- tests/unit/cmor/test_fix.py | 4 +- 7 files changed, 206 insertions(+), 269 deletions(-) diff --git a/esmvalcore/_main.py b/esmvalcore/_main.py index 3f482d36e5..a2339c7ce7 100755 --- a/esmvalcore/_main.py +++ b/esmvalcore/_main.py @@ -42,6 +42,7 @@ from ._config import configure_logging, read_config_user_file, DIAGNOSTICS_PATH from ._recipe import TASKSEP, read_recipe_file from ._task import resource_usage_logger +from .cmor.check import CheckLevels # set up logging logger = logging.getLogger(__name__) @@ -99,15 +100,22 @@ def get_args(): nargs='*', help="Only run the named diagnostics from the recipe.") parser.add_argument( - '--cmor-checks', + '--check-level', type=str, - choices=('none', 'relaxed', 'default'), - default='default', - help="Metadata CMOR checks strictness; \ - Optional: true; possible values: \ - none: no checks performed \ - relaxed: checks are performed but no errors raised; \ - default: full checks and errors raised.") + choices=[val.name for val in CheckLevels if val != CheckLevels.DEBUG], + default='ERROR', + help=""" + Configure the severity of the errors that will make the CMOR check + fail. + Optional: true; + Possible values: + IGNORE_ALL: all issues will be reported as debug messages + NEVER_FAIL: all errors will be reported as warnings + CRITICAL: only fail if there are critical errors + ERROR: fail if there are any errors (DEFAULT) + WARNING: fail if there are any warnings + """ + ) args = parser.parse_args() return args @@ -153,7 +161,7 @@ def main(args): pattern if TASKSEP in pattern else pattern + TASKSEP + '*' for pattern in args.diagnostics or () } - cfg['cmor_checks'] = args.cmor_checks + cfg['check_level'] = args.check_level cfg['synda_download'] = args.synda_download for limit in ('max_datasets', 'max_years'): value = getattr(args, limit) diff --git a/esmvalcore/_recipe.py b/esmvalcore/_recipe.py index 1ee9f8705c..7e56c69ec5 100644 --- a/esmvalcore/_recipe.py +++ b/esmvalcore/_recipe.py @@ -309,15 +309,7 @@ def _get_default_settings(variable, config_user, derive=False): fix['cmor_table'] = variable['cmor_table'] fix['mip'] = variable['mip'] fix['frequency'] = variable['frequency'] - cmor_check = config_user.get('cmor_checks', None) - raise_exception = True - report_only_warning = False - if cmor_check in ['none', 'relaxed']: - raise_exception = False - if cmor_check == 'relaxed': - report_only_warning = True - fix['raise_exception'] = raise_exception - fix['report_only_warning'] = report_only_warning + fix['check_level'] = config_user.get('check_level', None) settings['fix_data'] = dict(fix) settings['fix_metadata'] = dict(fix) @@ -342,20 +334,12 @@ def _get_default_settings(variable, config_user, derive=False): # Configure CMOR metadata check if variable.get('cmor_table'): - raise_exception = True - report_only_warning = False - cmor_check = config_user.get('cmor_checks', None) - if cmor_check in ['none', 'relaxed']: - raise_exception = False - if cmor_check == 'relaxed': - report_only_warning = True settings['cmor_check_metadata'] = { 'cmor_table': variable['cmor_table'], 'mip': variable['mip'], 'short_name': variable['short_name'], 'frequency': variable['frequency'], - 'raise_exception': raise_exception, - 'report_only_warning': report_only_warning, + 'check_level': config_user.get('check_level', None) } # Configure final CMOR data check if variable.get('cmor_table'): @@ -364,6 +348,7 @@ def _get_default_settings(variable, config_user, derive=False): 'mip': variable['mip'], 'short_name': variable['short_name'], 'frequency': variable['frequency'], + 'check_level': config_user.get('check_level', None) } # Clean up fixed files diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index e72d9b6f9b..a30c1bca49 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -1,5 +1,6 @@ """Module for checking iris cubes against their CMOR definitions.""" import logging +from enum import IntEnum import cf_units import iris.coord_categorisation @@ -10,6 +11,9 @@ from .table import CMOR_TABLES +CheckLevels = IntEnum( + 'CheckLevels', 'DEBUG WARNING ERROR CRITICAL NEVER_FAIL IGNORE_ALL') + class CMORCheckError(Exception): """Exception raised when a cube does not pass the CMORCheck.""" @@ -57,14 +61,13 @@ def __init__(self, var_info, frequency=None, fail_on_error=False, - raise_exception=True, - report_only_warning=False, + check_level=CheckLevels.ERROR, automatic_fixes=False): self._cube = cube self._failerr = fail_on_error - self._raise_exception = raise_exception - self._report_only_warning = report_only_warning + self._check_level = check_level + self._logger = logging.getLogger(__name__) self._errors = list() self._warnings = list() self._debug_messages = list() @@ -95,8 +98,8 @@ def check_metadata(self, logger=None): all checks and then raises. """ - if logger is None: - logger = logging.getLogger(__name__) + if logger is not None: + self._logger = logger self._check_var_metadata() self._check_fill_value() @@ -104,14 +107,47 @@ def check_metadata(self, logger=None): self._check_coords() if self.frequency != 'fx': self._check_time_coord() + self._check_rank() - self.report_debug_messages(logger) - self.report_warnings(logger) + self.report_debug_messages() + self.report_warnings() self.report_errors() if self.frequency != 'fx': self._add_auxiliar_time_coordinates() + + return self._cube + + def check_data(self, logger=None): + """Check the cube data. + + Performs all the tests that require to have the data in memory. + Assumes that metadata is correct, so you must call check_metadata prior + to this. + + It will also report some warnings in case of minor errors. + + Raises + ------ + CMORCheckError + If errors are found. If fail_on_error attribute is set to True, + raises as soon as an error is detected. If set to False, it perform + all checks and then raises. + + """ + if logger is not None: + self._logger = logger + + if self._cmor_var.units: + units = self._get_effective_units() + if str(self._cube.units) != units: + self._cube.convert_units(units) + + self._check_coords_data() + + self.report_warnings() + self.report_errors() return self._cube def report_errors(self): @@ -129,7 +165,7 @@ def report_errors(self): self._cube) raise CMORCheckError(msg) - def report_warnings(self, logger): + def report_warnings(self): """Report detected warnings to the given logger. Parameters @@ -140,9 +176,9 @@ def report_warnings(self, logger): if self.has_warnings(): msg = 'There were warnings in variable {}:\n{}\n'.format( self._cube.var_name, '\n '.join(self._warnings)) - logger.warning(msg) + self._logger.warning(msg) - def report_debug_messages(self, logger): + def report_debug_messages(self): """Report detected debug messages to the given logger. Parameters @@ -153,38 +189,7 @@ def report_debug_messages(self, logger): if self.has_debug_messages(): msg = 'There were metadata changes in variable {}:\n{}\n'.format( self._cube.var_name, '\n '.join(self._debug_messages)) - logger.debug(msg) - - def check_data(self, logger=None): - """Check the cube data. - - Performs all the tests that require to have the data in memory. - Assumes that metadata is correct, so you must call check_metadata prior - to this. - - It will also report some warnings in case of minor errors. - - Raises - ------ - CMORCheckError - If errors are found. If fail_on_error attribute is set to True, - raises as soon as an error is detected. If set to False, it perform - all checks and then raises. - - """ - if logger is None: - logger = logging.getLogger(__name__) - - if self._cmor_var.units: - units = self._get_effective_units() - if str(self._cube.units) != units: - self._cube.convert_units(units) - - self._check_coords_data() - - self.report_warnings(logger) - self.report_errors() - return self._cube + self._logger.debug(msg) def _check_fill_value(self): """Check fill value.""" @@ -208,7 +213,7 @@ def _check_var_metadata(self): ) self._cube.standard_name = self._cmor_var.standard_name else: - self.report_strict_error( + self.report_error( self._attr_msg, self._cube.var_name, 'standard_name', self._cmor_var.standard_name, self._cube.standard_name ) @@ -224,7 +229,7 @@ def _check_var_metadata(self): ) self._cube.long_name = self._cmor_var.long_name else: - self.report_strict_error( + self.report_error( self._attr_msg, self._cube.var_name, 'long_name', self._cmor_var.long_name, self._cube.long_name ) @@ -239,7 +244,7 @@ def _check_var_metadata(self): units = self._get_effective_units() if not self._cube.units.is_convertible(units): - self.report_strict_error( + self.report_error( f'Variable {self._cube.var_name} units ' f'{self._cube.units} can not be ' f'converted to {self._cmor_var.units}') @@ -253,7 +258,7 @@ def _check_var_metadata(self): self.report_warning('{}: attribute {} not present', self._cube.var_name, attr) elif self._cube.attributes[attr] != attr_value: - self.report_strict_error( + self.report_error( self._attr_msg, self._cube.var_name, attr, attr_value, self._cube.attributes[attr]) @@ -284,8 +289,8 @@ def _check_rank(self): # Check number of dimension coords matches rank if self._cube.ndim != rank: - self.report_strict_error(self._does_msg, self._cube.var_name, - 'match coordinate rank') + self.report_error(self._does_msg, self._cube.var_name, + 'match coordinate rank') def _check_dim_names(self): """Check dimension names.""" @@ -296,7 +301,7 @@ def _check_dim_names(self): try: cube_coord = self._cube.coord(var_name=coordinate.out_name) if cube_coord.standard_name != coordinate.standard_name: - self.report_error( + self.report_critical( self._attr_msg, coordinate.out_name, 'standard_name', @@ -320,7 +325,7 @@ def _check_dim_names(self): ) coord.var_name = coordinate.out_name else: - self.report_strict_error( + self.report_error( 'Coordinate {0} has var name {1}' 'instead of {2}', coordinate.name, @@ -330,12 +335,11 @@ def _check_dim_names(self): except iris.exceptions.CoordinateNotFoundError: if coordinate.standard_name in ['time', 'latitude', 'longitude']: - self.report_error(self._does_msg, coordinate.name, - 'exist') + self.report_critical( + self._does_msg, coordinate.name, 'exist') else: - self.report_strict_error(self._does_msg, - coordinate.name, - 'exist') + self.report_error( + self._does_msg, coordinate.name, 'exist') def _check_coords(self): """Check coordinates.""" @@ -386,28 +390,30 @@ def _check_coord(self, cmor, coord, var_name): except ValueError: pass if not fixed: - self.report_error(self._attr_msg, var_name, 'units', - cmor.units, coord.units) + self.report_critical(self._attr_msg, var_name, 'units', + cmor.units, coord.units) self._check_coord_values(cmor, coord, var_name) self._check_coord_monotonicity_and_direction(cmor, coord, var_name) def _check_coord_monotonicity_and_direction(self, cmor, coord, var_name): """Check monotonicity and direction of coordinate.""" if not coord.is_monotonic(): - self.report_error(self._is_msg, var_name, 'monotonic') + self.report_critical(self._is_msg, var_name, 'monotonic') if len(coord.points) == 1: return if cmor.stored_direction: if cmor.stored_direction == 'increasing': if coord.points[0] > coord.points[1]: if not self.automatic_fixes or coord.ndim > 1: - self.report_error(self._is_msg, var_name, 'increasing') + self.report_critical( + self._is_msg, var_name, 'increasing') else: self._reverse_coord(coord) elif cmor.stored_direction == 'decreasing': if coord.points[0] < coord.points[1]: if not self.automatic_fixes or coord.ndim > 1: - self.report_error(self._is_msg, var_name, 'decreasing') + self.report_critical( + self._is_msg, var_name, 'decreasing') else: self._reverse_coord(coord) @@ -432,8 +438,9 @@ def _check_coord_values(self, coord_info, coord, var_name): self.automatic_fixes: l_fix_coord_value = True else: - self.report_error(self._vals_msg, var_name, - '< {} ='.format('valid_min'), valid_min) + self.report_critical( + self._vals_msg, var_name, + '< {} ='.format('valid_min'), valid_min) if coord_info.valid_max: valid_max = float(coord_info.valid_max) @@ -442,8 +449,9 @@ def _check_coord_values(self, coord_info, coord, var_name): self.automatic_fixes: l_fix_coord_value = True else: - self.report_error(self._vals_msg, var_name, - '> {} ='.format('valid_max'), valid_max) + self.report_critical( + self._vals_msg, var_name, + '> {} ='.format('valid_max'), valid_max) if l_fix_coord_value: lon_extent = iris.coords.CoordExtent(coord, 0.0, 360., True, False) @@ -471,13 +479,13 @@ def _check_time_coord(self): var_name = coord.var_name if not coord.is_monotonic(): - self.report_strict_error( + self.report_error( 'Time coordinate for var {} is not monotonic', var_name ) if not coord.units.is_time_reference(): - self.report_error(self._does_msg, var_name, - 'have time reference units') + self.report_critical(self._does_msg, var_name, + 'have time reference units') else: old_units = coord.units coord.convert_units( @@ -532,7 +540,7 @@ def _check_time_coord(self): if second_month != second.month or \ second_year != second.year: msg = '{}: Frequency {} does not match input data' - self.report_strict_error(msg, var_name, freq) + self.report_error(msg, var_name, freq) break elif freq == 'yr': for i in range(len(coord.points) - 1): @@ -541,7 +549,7 @@ def _check_time_coord(self): second_month = first.month + 1 if first.year + 1 != second.year: msg = '{}: Frequency {} does not match input data' - self.report_strict_error(msg, var_name, freq) + self.report_error(msg, var_name, freq) break else: if freq in intervals: @@ -557,14 +565,14 @@ def _check_time_coord(self): target_interval = (frequency - tol, frequency + tol) else: msg = '{}: Frequency {} not supported by checker' - self.report_strict_error(msg, var_name, freq) + self.report_error(msg, var_name, freq) return for i in range(len(coord.points) - 1): interval = coord.points[i + 1] - coord.points[i] if (interval < target_interval[0] or interval > target_interval[1]): msg = '{}: Frequency {} does not match input data' - self.report_strict_error(msg, var_name, freq) + self.report_error(msg, var_name, freq) break @staticmethod @@ -609,7 +617,25 @@ def has_debug_messages(self): """ return len(self._debug_messages) > 0 - def report_error(self, message, *args): + def report(self, level, message, *args): + msg = message.format(*args) + if (level == CheckLevels.DEBUG or + self._check_level == CheckLevels.IGNORE_ALL): + if self._failerr: + self._logger.debug(msg) + else: + self._debug_messages.append(msg) + elif level < self._check_level: + if self._failerr: + self._logger.warning(msg) + else: + self._warnings.append(msg) + else: + if self._failerr: + raise CMORCheckError(msg + '\nin cube:\n{}'.format(self._cube)) + self._errors.append(msg) + + def report_critical(self, message, *args): """Report an error. If fail_on_error is set to True, raises automatically. @@ -623,16 +649,10 @@ def report_error(self, message, *args): arguments to format the message string. """ - msg = message.format(*args) - if self._failerr: - raise CMORCheckError(msg + '\nin cube:\n{}'.format(self._cube)) - self._errors.append(msg) + self.report(CheckLevels.CRITICAL, message, *args) - def report_strict_error(self, message, *args): - """Report a STRICT error. - If raise_exception True report error - If raise_exception False and report_only_warning True report_warning. - If raise_exception False and report_only_warning False do not report. + def report_error(self, message, *args): + """Report a normal error. Parameters ---------- @@ -642,16 +662,10 @@ def report_strict_error(self, message, *args): arguments to format the message string. """ - if self._raise_exception: - self.report_error(message, *args) - if self._report_only_warning: - self.report_warning(message, *args) + self.report(CheckLevels.ERROR, message, *args) def report_warning(self, message, *args): - """Report a warning. - - If fail_on_error is set to True, logs it automatically. - If fail_on_error is set to False, stores it for later reports. + """Report a warning level error. Parameters ---------- @@ -661,11 +675,7 @@ def report_warning(self, message, *args): arguments to format the message string. """ - msg = message.format(*args) - if self._failerr: - print('WARNING: {0}'.format(msg)) - else: - self._warnings.append(msg) + self.report(CheckLevels.WARNING, message, *args) def report_debug_message(self, message, *args): """Report a debug message. @@ -678,8 +688,7 @@ def report_debug_message(self, message, *args): arguments to format the message string """ - msg = message.format(*args) - self._debug_messages.append(msg) + self.report(CheckLevels.DEBUG, message, *args) def _add_auxiliar_time_coordinates(self): coords = [coord.name() for coord in self._cube.aux_coords] @@ -698,8 +707,7 @@ def _get_cmor_checker(table, short_name, frequency, fail_on_error=False, - raise_exception=True, - report_only_warning=False, + check_level=CheckLevels.ERROR, automatic_fixes=False): """Get a CMOR checker/fixer.""" if table not in CMOR_TABLES: @@ -719,8 +727,7 @@ def _checker(cube): var_info, frequency=frequency, fail_on_error=fail_on_error, - raise_exception=raise_exception, - report_only_warning=report_only_warning, + check_level=check_level, automatic_fixes=automatic_fixes) return _checker @@ -728,7 +735,7 @@ def _checker(cube): def cmor_check_metadata(cube, cmor_table, mip, short_name, frequency, - raise_exception, report_only_warning): + check_level): """Check if metadata conforms to variable's CMOR definiton. None of the checks at this step will force the cube to load the data. @@ -745,20 +752,16 @@ def cmor_check_metadata(cube, cmor_table, mip, Variable's short name. frequency: basestring Data frequency. - raise_exception: bool - Boolean operator that raises or not - the exception resulted from checker. """ checker = _get_cmor_checker(cmor_table, mip, short_name, frequency, - raise_exception=raise_exception, - report_only_warning=report_only_warning) + check_level=check_level) checker(cube).check_metadata() return cube -def cmor_check_data(cube, cmor_table, mip, short_name, frequency): +def cmor_check_data(cube, cmor_table, mip, short_name, frequency, check_level): """Check if data conforms to variable's CMOR definiton. The checks performed at this step require the data in memory. @@ -777,14 +780,13 @@ def cmor_check_data(cube, cmor_table, mip, short_name, frequency): Data frequency """ - checker = _get_cmor_checker(cmor_table, mip, short_name, frequency) + checker = _get_cmor_checker(cmor_table, mip, short_name, frequency, + check_level=check_level) checker(cube).check_data() return cube -def cmor_check(cube, cmor_table, mip, short_name, frequency, - raise_exception, - report_only_warning): +def cmor_check(cube, cmor_table, mip, short_name, frequency, check_level): """Check if cube conforms to variable's CMOR definiton. Equivalent to calling cmor_check_metadata and cmor_check_data @@ -805,7 +807,6 @@ def cmor_check(cube, cmor_table, mip, short_name, frequency, """ cmor_check_metadata(cube, cmor_table, mip, short_name, frequency, - raise_exception=raise_exception, - report_only_warning=report_only_warning) + check_level=check_level) cmor_check_data(cube, cmor_table, mip, short_name, frequency) return cube diff --git a/esmvalcore/cmor/fix.py b/esmvalcore/cmor/fix.py index 047163442c..e50bbe3941 100644 --- a/esmvalcore/cmor/fix.py +++ b/esmvalcore/cmor/fix.py @@ -13,7 +13,7 @@ from iris.cube import CubeList from ._fixes.fix import Fix -from .check import _get_cmor_checker +from .check import _get_cmor_checker, CheckLevels logger = logging.getLogger(__name__) @@ -57,8 +57,7 @@ def fix_metadata(cubes, cmor_table=None, mip=None, frequency=None, - raise_exception=True, - report_only_warning=False, ): + check_level=CheckLevels.ERROR): """ Fix cube metadata if fixes are required and check it anyway. @@ -142,8 +141,7 @@ def fix_metadata(cubes, table=cmor_table, mip=mip, short_name=short_name, - raise_exception=raise_exception, - report_only_warning=report_only_warning, + check_level=check_level, fail_on_error=False, automatic_fixes=True) cube = checker(cube).check_metadata() @@ -159,8 +157,7 @@ def fix_data(cube, cmor_table=None, mip=None, frequency=None, - raise_exception=True, - report_only_warning=False, ): + check_level=CheckLevels.ERROR): """ Fix cube data if fixes add present and check it anyway. diff --git a/tests/integration/test_recipe.py b/tests/integration/test_recipe.py index cde3b6d2e6..963a3df049 100644 --- a/tests/integration/test_recipe.py +++ b/tests/integration/test_recipe.py @@ -14,6 +14,7 @@ from esmvalcore._task import DiagnosticTask from esmvalcore.preprocessor import DEFAULT_ORDER, PreprocessingTask from esmvalcore.preprocessor._io import concatenate_callback +from esmvalcore.cmor.check import CheckLevels from .test_diagnostic_run import write_config_user_file from .test_provenance import check_provenance @@ -333,24 +334,22 @@ def test_default_preprocessor(tmp_path, patched_datafinder, config_user): 'output_dir': fix_dir, }, 'fix_data': { + 'check_level': None, 'project': 'CMIP5', 'dataset': 'CanESM2', 'short_name': 'chl', 'cmor_table': 'CMIP5', 'mip': 'Oyr', 'frequency': 'yr', - 'raise_exception': True, - 'report_only_warning': False, }, 'fix_metadata': { + 'check_level': None, 'project': 'CMIP5', 'dataset': 'CanESM2', 'short_name': 'chl', 'cmor_table': 'CMIP5', 'mip': 'Oyr', 'frequency': 'yr', - 'raise_exception': True, - 'report_only_warning': False, }, 'extract_time': { 'start_year': 2000, @@ -361,14 +360,14 @@ def test_default_preprocessor(tmp_path, patched_datafinder, config_user): 'end_day': 1, }, 'cmor_check_metadata': { + 'check_level': None, 'cmor_table': 'CMIP5', 'mip': 'Oyr', 'short_name': 'chl', 'frequency': 'yr', - 'raise_exception': True, - 'report_only_warning': False, }, 'cmor_check_data': { + 'check_level': None, 'cmor_table': 'CMIP5', 'mip': 'Oyr', 'short_name': 'chl', @@ -426,34 +425,32 @@ def test_default_fx_preprocessor(tmp_path, patched_datafinder, config_user): 'output_dir': fix_dir, }, 'fix_data': { + 'check_level': None, 'project': 'CMIP5', 'dataset': 'CanESM2', 'short_name': 'sftlf', 'cmor_table': 'CMIP5', 'mip': 'fx', 'frequency': 'fx', - 'raise_exception': True, - 'report_only_warning': False, }, 'fix_metadata': { + 'check_level': None, 'project': 'CMIP5', 'dataset': 'CanESM2', 'short_name': 'sftlf', 'cmor_table': 'CMIP5', 'mip': 'fx', 'frequency': 'fx', - 'raise_exception': True, - 'report_only_warning': False, }, 'cmor_check_metadata': { + 'check_level': None, 'cmor_table': 'CMIP5', 'mip': 'fx', 'short_name': 'sftlf', 'frequency': 'fx', - 'raise_exception': True, - 'report_only_warning': False, }, 'cmor_check_data': { + 'check_level': None, 'cmor_table': 'CMIP5', 'mip': 'fx', 'short_name': 'sftlf', diff --git a/tests/unit/cmor/test_cmor_check.py b/tests/unit/cmor/test_cmor_check.py index 964f14aee0..a139292ee7 100644 --- a/tests/unit/cmor/test_cmor_check.py +++ b/tests/unit/cmor/test_cmor_check.py @@ -1,8 +1,6 @@ """Unit tests for the CMORCheck class.""" -import sys import unittest -from io import StringIO import iris import iris.coord_categorisation @@ -11,7 +9,7 @@ import numpy as np from cf_units import Unit -from esmvalcore.cmor.check import CMORCheck, CMORCheckError +from esmvalcore.cmor.check import CMORCheck, CMORCheckError, CheckLevels class VariableInfoMock: @@ -96,14 +94,14 @@ def test_report_error(self): """Test report error function.""" checker = CMORCheck(self.cube, self.var_info) self.assertFalse(checker.has_errors()) - checker.report_error('New error: {}', 'something failed') + checker.report_critical('New error: {}', 'something failed') self.assertTrue(checker.has_errors()) def test_fail_on_error(self): """Test exception is raised if fail_on_error is activated.""" checker = CMORCheck(self.cube, self.var_info, fail_on_error=True) with self.assertRaises(CMORCheckError): - checker.report_error('New error: {}', 'something failed') + checker.report_critical('New error: {}', 'something failed') def test_report_warning(self): """Test report warning function.""" @@ -115,12 +113,12 @@ def test_report_warning(self): def test_warning_fail_on_error(self): """Test report warning function with fail_on_error.""" checker = CMORCheck(self.cube, self.var_info, fail_on_error=True) - stdout = sys.stdout - sys.stdout = StringIO() - checker.report_warning('New error: {}', 'something failed') - output = sys.stdout.getvalue().strip() - sys.stdout = stdout - self.assertEqual(output, 'WARNING: New error: something failed') + with self.assertLogs(level='WARNING') as cm: + checker.report_warning('New error: {}', 'something failed') + self.assertEqual( + cm.output, + ['WARNING:esmvalcore.cmor.check:New error: something failed', ] + ) def test_report_debug_message(self): """"Test report debug message function""" @@ -134,7 +132,7 @@ def test_check(self): self._check_cube() def _check_cube(self, automatic_fixes=False, frequency=None, - raise_exception=True, report_only_warning=False): + check_level=CheckLevels.ERROR): """Apply checks and optionally automatic fixes to self.cube.""" def checker(cube): return CMORCheck( @@ -142,14 +140,13 @@ def checker(cube): self.var_info, automatic_fixes=automatic_fixes, frequency=frequency, - raise_exception=raise_exception, - report_only_warning=report_only_warning) + check_level=check_level) self.cube = checker(self.cube).check_metadata() self.cube = checker(self.cube).check_data() def _check_cube_metadata(self, automatic_fixes=False, frequency=None, - raise_exception=True, report_only_warning=False): + check_level=CheckLevels.ERROR): """Apply checks and optionally automatic fixes to self.cube.""" def checker(cube): return CMORCheck( @@ -157,8 +154,7 @@ def checker(cube): self.var_info, automatic_fixes=automatic_fixes, frequency=frequency, - raise_exception=raise_exception, - report_only_warning=report_only_warning) + check_level=check_level) self.cube = checker(self.cube).check_metadata() @@ -280,27 +276,21 @@ def test_check_bad_var_standard_name_strict_flag(self): --cmor-check strict.""" self.cube = self.get_cube(self.var_info) self.cube.standard_name = 'wind_speed' - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=True, - report_only_warning=False) + self._check_fails_in_metadata(automatic_fixes=False) def test_check_bad_var_long_name_strict_flag(self): """Test check fails for a bad variable long_name with --cmor-check strict.""" self.cube = self.get_cube(self.var_info) self.cube.long_name = "Near-Surface Wind Speed" - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=True, - report_only_warning=False) + self._check_fails_in_metadata(automatic_fixes=False) def test_check_bad_var_units_strict_flag(self): """Test check fails for a bad variable units with --cmor-check strict.""" self.cube = self.get_cube(self.var_info) self.cube.units = "kg" - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=True, - report_only_warning=False) + self._check_fails_in_metadata(automatic_fixes=False) def test_check_bad_attributes_strict_flag(self): """Test check fails for a bad variable attribute with @@ -309,9 +299,7 @@ def test_check_bad_attributes_strict_flag(self): self.var_info.positive = "up" self.cube = self.get_cube(self.var_info) self.cube.attributes['positive'] = "Wrong attribute" - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=True, - report_only_warning=False) + self._check_fails_in_metadata(automatic_fixes=False) def test_check_bad_rank_strict_flag(self): """Test check fails for a bad variable rank with @@ -319,52 +307,40 @@ def test_check_bad_rank_strict_flag(self): lat = iris.coords.AuxCoord.from_coord(self.cube.coord('latitude')) self.cube.remove_coord('latitude') self.cube.add_aux_coord(lat, self.cube.coord_dims('longitude')) - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=True, - report_only_warning=False) + self._check_fails_in_metadata(automatic_fixes=False) def test_check_bad_coord_var_name_strict_flag(self): """Test check fails for bad coord var_name with --cmor-check strict""" self.var_info.table_type = 'CMIP5' self.cube.coord('longitude').var_name = 'bad_name' - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=True, - report_only_warning=False) + self._check_fails_in_metadata(automatic_fixes=False) def test_check_missing_lon_strict_flag(self): """Test check fails for missing longitude with --cmor-check strict""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('longitude') - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=True, - report_only_warning=False) + self._check_fails_in_metadata(automatic_fixes=False) def test_check_missing_lat_strict_flag(self): """Test check fails for missing latitude with --cmor-check strict""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('latitude') - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=True, - report_only_warning=False) + self._check_fails_in_metadata(automatic_fixes=False) def test_check_missing_time_strict_flag(self): """Test check fails for missing time with --cmor-check strict""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('time') - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=True, - report_only_warning=False) + self._check_fails_in_metadata(automatic_fixes=False) def test_check_missing_coord_strict_flag(self): """Test check fails for missing coord other than lat and lon with --cmor-check strict""" self.cube = self.get_cube(self.var_info) self.var_info.coordinates = {'height2m': CoordinateInfoMock('height2m') - } - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=True, - report_only_warning=False) + } + self._check_fails_in_metadata(automatic_fixes=False) def test_check_bad_var_standard_name_relaxed_flag(self): """Test check reports warning for a bad variable standard_name with @@ -372,8 +348,7 @@ def test_check_bad_var_standard_name_relaxed_flag(self): self.cube = self.get_cube(self.var_info) self.cube.standard_name = 'wind_speed' self._check_warnings_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=True) + check_level=CheckLevels.CRITICAL) def test_check_bad_var_long_name_relaxed_flag(self): """Test check reports warning for a bad variable long_name with @@ -381,8 +356,7 @@ def test_check_bad_var_long_name_relaxed_flag(self): self.cube = self.get_cube(self.var_info) self.cube.long_name = "Near-Surface Wind Speed" self._check_warnings_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=True) + check_level=CheckLevels.CRITICAL) def test_check_bad_var_units_relaxed_flag(self): """Test check reports warning for a bad variable units with @@ -390,8 +364,7 @@ def test_check_bad_var_units_relaxed_flag(self): self.cube = self.get_cube(self.var_info) self.cube.units = "kg" self._check_warnings_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=True) + check_level=CheckLevels.CRITICAL) def test_check_bad_attributes_relaxed_flag(self): """Test check report warnings for a bad variable attribute with @@ -401,8 +374,7 @@ def test_check_bad_attributes_relaxed_flag(self): self.cube = self.get_cube(self.var_info) self.cube.attributes['positive'] = "Wrong attribute" self._check_warnings_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=True) + check_level=CheckLevels.CRITICAL) def test_check_bad_rank_relaxed_flag(self): """Test check report warnings for a bad variable rank with @@ -411,8 +383,7 @@ def test_check_bad_rank_relaxed_flag(self): self.cube.remove_coord('latitude') self.cube.add_aux_coord(lat, self.cube.coord_dims('longitude')) self._check_warnings_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=True) + check_level=CheckLevels.CRITICAL) def test_check_bad_coord_standard_name_relaxed_flag(self): """Test check reports warning for bad coord var_name with @@ -420,42 +391,37 @@ def test_check_bad_coord_standard_name_relaxed_flag(self): self.var_info.table_type = 'CMIP5' self.cube.coord('longitude').var_name = 'bad_name' self._check_warnings_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=True) + check_level=CheckLevels.CRITICAL) def test_check_missing_lon_relaxed_flag(self): """Test check fails for missing longitude with --cmor-check relaxed""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('longitude') self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=True) + check_level=CheckLevels.CRITICAL) def test_check_missing_lat_relaxed_flag(self): """Test check fails for missing latitude with --cmor-check relaxed""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('latitude') self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=True) + check_level=CheckLevels.CRITICAL) def test_check_missing_time_relaxed_flag(self): """Test check fails for missing latitude with --cmor-check relaxed""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('time') self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=True) + check_level=CheckLevels.CRITICAL) def test_check_missing_coord_relaxed_flag(self): """Test check reports warning for missing coord other than lat and lon with --cmor-check relaxed""" self.cube = self.get_cube(self.var_info) self.var_info.coordinates = {'height2m': CoordinateInfoMock('height2m') - } + } self._check_warnings_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=True) + check_level=CheckLevels.CRITICAL) def test_check_bad_var_standard_name_none_flag(self): """Test check does not report for a bad variable standard_name with @@ -463,8 +429,7 @@ def test_check_bad_var_standard_name_none_flag(self): self.cube = self.get_cube(self.var_info) self.cube.standard_name = 'wind_speed' self._check_no_reports_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=False) + check_level=CheckLevels.IGNORE_ALL) def test_check_bad_var_long_name_none_flag(self): """Test check does not report for a bad variable long_name with @@ -472,8 +437,7 @@ def test_check_bad_var_long_name_none_flag(self): self.cube = self.get_cube(self.var_info) self.cube.long_name = "Near-Surface Wind Speed" self._check_no_reports_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=False) + check_level=CheckLevels.IGNORE_ALL) def test_check_bad_var_units_none_flag(self): """Test check does not report for a bad variable unit with @@ -481,8 +445,7 @@ def test_check_bad_var_units_none_flag(self): self.cube = self.get_cube(self.var_info) self.cube.units = "kg" self._check_no_reports_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=False) + check_level=CheckLevels.IGNORE_ALL) def test_check_bad_attributes_none_flag(self): """Test check does not report for a bad variable attribute with @@ -492,8 +455,7 @@ def test_check_bad_attributes_none_flag(self): self.cube = self.get_cube(self.var_info) self.cube.attributes['positive'] = "Wrong attribute" self._check_no_reports_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=False) + check_level=CheckLevels.IGNORE_ALL) def test_check_bad_rank_none_flag(self): """Test check does not report for a bad variable rank with @@ -501,9 +463,8 @@ def test_check_bad_rank_none_flag(self): lat = iris.coords.AuxCoord.from_coord(self.cube.coord('latitude')) self.cube.remove_coord('latitude') self.cube.add_aux_coord(lat, self.cube.coord_dims('longitude')) - self._check_no_reports_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=False) + self._check_warnings_on_metadata(automatic_fixes=False, + check_level=CheckLevels.CRITICAL) def test_check_bad_coord_standard_name_none_flag(self): """Test check reports nothing for bad coord var_name with @@ -511,75 +472,63 @@ def test_check_bad_coord_standard_name_none_flag(self): self.var_info.table_type = 'CMIP5' self.cube.coord('longitude').var_name = 'bad_name' self._check_no_reports_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=False) + check_level=CheckLevels.IGNORE_ALL) def test_check_missing_lon_none_flag(self): """Test check fails for missing longitude with --cmor-check none""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('longitude') - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=False) + self._check_no_reports_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE_ALL) def test_check_missing_lat_none_flag(self): """Test check fails for missing latitude with --cmor-check none""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('latitude') - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=False) + self._check_no_reports_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE_ALL) def test_check_missing_time_none_flag(self): """Test check fails for missing time with --cmor-check none""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('time') - self._check_fails_in_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=False) + self._check_no_reports_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE_ALL) def test_check_missing_coord_none_flag(self): """Test check reports nothing for missing coord other than lat, lon and time with --cmor-check none""" self.cube = self.get_cube(self.var_info) self.var_info.coordinates = {'height2m': CoordinateInfoMock('height2m') - } + } self._check_no_reports_on_metadata(automatic_fixes=False, - raise_exception=False, - report_only_warning=False) - + check_level=CheckLevels.IGNORE_ALL) def _check_fails_in_metadata(self, automatic_fixes=False, frequency=None, - raise_exception=True, - report_only_warning=False): + check_level=CheckLevels.ERROR): checker = CMORCheck( self.cube, self.var_info, automatic_fixes=automatic_fixes, frequency=frequency, - raise_exception=raise_exception, - report_only_warning=report_only_warning) + check_level=check_level) with self.assertRaises(CMORCheckError): checker.check_metadata() def _check_warnings_on_metadata(self, automatic_fixes=False, - raise_exception=True, - report_only_warning=False): + check_level=CheckLevels.ERROR): checker = CMORCheck( self.cube, self.var_info, automatic_fixes=automatic_fixes, - raise_exception=raise_exception, - report_only_warning=report_only_warning + check_level=check_level ) checker.check_metadata() self.assertTrue(checker.has_warnings()) def _check_no_reports_on_metadata(self, automatic_fixes=False, - raise_exception=False, - report_only_warning=False): + check_level=CheckLevels.ERROR): checker = CMORCheck( self.cube, self.var_info, automatic_fixes=automatic_fixes, - raise_exception=raise_exception, - report_only_warning=report_only_warning + check_level=check_level ) checker.check_metadata() self.assertFalse(checker.has_errors()) diff --git a/tests/unit/cmor/test_fix.py b/tests/unit/cmor/test_fix.py index 48cfe8dda3..f041e4d8c4 100644 --- a/tests/unit/cmor/test_fix.py +++ b/tests/unit/cmor/test_fix.py @@ -5,6 +5,7 @@ import mock from esmvalcore.cmor.fix import Fix, fix_data, fix_file, fix_metadata +from esmvalcore.cmor.check import CheckLevels class TestFixFile(unittest.TestCase): @@ -150,8 +151,7 @@ def test_cmor_checker_called(self): mip='mip', short_name='short_name', table='cmor_table', - raise_exception=True, - report_only_warning=False) + check_level=CheckLevels.ERROR,) checker.assert_called_once_with(self.cube) checker.return_value.check_metadata.assert_called_once_with() From 6addf1fd355a2f341cb8ab877725cca0821cf849 Mon Sep 17 00:00:00 2001 From: Javier Vegas-Regidor Date: Tue, 3 Dec 2019 11:43:02 +0100 Subject: [PATCH 30/59] Remove unused import --- tests/integration/test_recipe.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/test_recipe.py b/tests/integration/test_recipe.py index 963a3df049..74316788dc 100644 --- a/tests/integration/test_recipe.py +++ b/tests/integration/test_recipe.py @@ -14,7 +14,6 @@ from esmvalcore._task import DiagnosticTask from esmvalcore.preprocessor import DEFAULT_ORDER, PreprocessingTask from esmvalcore.preprocessor._io import concatenate_callback -from esmvalcore.cmor.check import CheckLevels from .test_diagnostic_run import write_config_user_file from .test_provenance import check_provenance From c55a1edd2d2a8a5827a52af6ecae77117eea5bb8 Mon Sep 17 00:00:00 2001 From: Javier Vegas-Regidor Date: Tue, 3 Dec 2019 11:53:38 +0100 Subject: [PATCH 31/59] Fix codacy issues --- esmvalcore/cmor/check.py | 20 +++++++++++++- esmvalcore/cmor/fix.py | 59 ++++++++++++++++++++++------------------ 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index a30c1bca49..02698c5286 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -618,6 +618,23 @@ def has_debug_messages(self): return len(self._debug_messages) > 0 def report(self, level, message, *args): + """Generic method to report a message from the checker + + Parameters + ---------- + level : CheckLevels + Message level + message : str + Message to report + args : + String format args for the message + + Raises + ------ + CMORCheckError + If fail on error is set, it is thrown when registering an error + message + """ msg = message.format(*args) if (level == CheckLevels.DEBUG or self._check_level == CheckLevels.IGNORE_ALL): @@ -808,5 +825,6 @@ def cmor_check(cube, cmor_table, mip, short_name, frequency, check_level): """ cmor_check_metadata(cube, cmor_table, mip, short_name, frequency, check_level=check_level) - cmor_check_data(cube, cmor_table, mip, short_name, frequency) + cmor_check_data(cube, cmor_table, mip, short_name, frequency, + check_level=check_level) return cube diff --git a/esmvalcore/cmor/fix.py b/esmvalcore/cmor/fix.py index e50bbe3941..0ac1d683f6 100644 --- a/esmvalcore/cmor/fix.py +++ b/esmvalcore/cmor/fix.py @@ -108,32 +108,7 @@ def fix_metadata(cubes, for fix in fixes: cube_list = fix.fix_metadata(cube_list) - if len(cube_list) != 1: - cube = None - for raw_cube in cube_list: - if raw_cube.var_name == short_name: - cube = raw_cube - break - if not cube: - raise ValueError( - 'More than one cube found for variable %s in %s:%s but ' - 'none of their var_names match the expected. \n' - 'Full list of cubes encountered: %s' % - (short_name, project, dataset, cube_list) - ) - logger.warning( - 'Found variable %s in %s:%s, but there were other present in ' - 'the file. Those extra variables are usually metadata ' - '(cell area, latitude descriptions) that was not saved ' - 'properly. It is possible that errors appear further on ' - 'because of this. \nFull list of cubes encountered: %s', - short_name, - project, - dataset, - cube_list - ) - else: - cube = cube_list[0] + cube = _get_single_cube(cube_list, short_name, project, dataset) if cmor_table and mip: checker = _get_cmor_checker( @@ -150,6 +125,35 @@ def fix_metadata(cubes, return fixed_cubes +def _get_single_cube(cube_list, short_name, project, dataset): + if len(cube_list) == 1: + return cube_list[0] + cube = None + for raw_cube in cube_list: + if raw_cube.var_name == short_name: + cube = raw_cube + break + if not cube: + raise ValueError( + 'More than one cube found for variable %s in %s:%s but ' + 'none of their var_names match the expected. \n' + 'Full list of cubes encountered: %s' % + (short_name, project, dataset, cube_list) + ) + logger.warning( + 'Found variable %s in %s:%s, but there were other present in ' + 'the file. Those extra variables are usually metadata ' + '(cell area, latitude descriptions) that was not saved ' + 'properly. It is possible that errors appear further on ' + 'because of this. \nFull list of cubes encountered: %s', + short_name, + project, + dataset, + cube_list + ) + return cube + + def fix_data(cube, short_name, project, @@ -208,6 +212,7 @@ def fix_data(cube, mip=mip, short_name=short_name, fail_on_error=False, - automatic_fixes=True) + automatic_fixes=True, + check_level=check_level) cube = checker(cube).check_data() return cube From ecc1062b6a41861815a010fb1353ecc082d412d8 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Tue, 3 Dec 2019 12:03:55 +0000 Subject: [PATCH 32/59] fixed func attr --- esmvalcore/cmor/check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 02698c5286..761d624bba 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -642,7 +642,7 @@ def report(self, level, message, *args): self._logger.debug(msg) else: self._debug_messages.append(msg) - elif level < self._check_level: + elif level < getattr(CheckLevels, self._check_level): if self._failerr: self._logger.warning(msg) else: From aa48d928c5e02e20568b8923958309262a3b9fd6 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Tue, 3 Dec 2019 14:06:26 +0000 Subject: [PATCH 33/59] fixed mock test --- tests/unit/cmor/test_fix.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/cmor/test_fix.py b/tests/unit/cmor/test_fix.py index f041e4d8c4..c9eed61c48 100644 --- a/tests/unit/cmor/test_fix.py +++ b/tests/unit/cmor/test_fix.py @@ -198,6 +198,7 @@ def test_cmor_checker_called(self): 'cmor_table', 'mip', 'frequency') get_mock.assert_called_once_with( automatic_fixes=True, + check_level=CheckLevels.ERROR, fail_on_error=False, frequency='frequency', mip='mip', From 70f64ce5db7729ab46bc1d45f63bfd8b6d097f3a Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Tue, 3 Dec 2019 14:06:47 +0000 Subject: [PATCH 34/59] proper fixed attr case is string --- esmvalcore/cmor/check.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 761d624bba..a90ae08f06 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -636,13 +636,16 @@ def report(self, level, message, *args): message """ msg = message.format(*args) + actual_level = self._check_level + if isinstance(actual_level, str): + actual_level = getattr(CheckLevels, self._check_level) if (level == CheckLevels.DEBUG or - self._check_level == CheckLevels.IGNORE_ALL): + actual_level == CheckLevels.IGNORE_ALL): if self._failerr: self._logger.debug(msg) else: self._debug_messages.append(msg) - elif level < getattr(CheckLevels, self._check_level): + elif level < actual_level: if self._failerr: self._logger.warning(msg) else: From f8584ce5b85e35b19a9e69670f4600d44542d273 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Tue, 3 Dec 2019 14:28:42 +0000 Subject: [PATCH 35/59] fixed typo in func name --- esmvalcore/cmor/check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index a90ae08f06..6e91dc40c2 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -115,7 +115,7 @@ def check_metadata(self, logger=None): self.report_errors() if self.frequency != 'fx': - self._add_auxiliar_time_coordinates() + self._add_auxiliary_time_coordinates() return self._cube @@ -710,7 +710,7 @@ def report_debug_message(self, message, *args): """ self.report(CheckLevels.DEBUG, message, *args) - def _add_auxiliar_time_coordinates(self): + def _add_auxiliary_time_coordinates(self): coords = [coord.name() for coord in self._cube.aux_coords] if 'day_of_month' not in coords: iris.coord_categorisation.add_day_of_month(self._cube, 'time') From a5885daff9f4bc6fe4a3d9b2a60c613949c449da Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Tue, 3 Dec 2019 14:31:37 +0000 Subject: [PATCH 36/59] add time aux coords only if time in coords --- esmvalcore/cmor/check.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 6e91dc40c2..bf5b3a273e 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -712,14 +712,15 @@ def report_debug_message(self, message, *args): def _add_auxiliary_time_coordinates(self): coords = [coord.name() for coord in self._cube.aux_coords] - if 'day_of_month' not in coords: - iris.coord_categorisation.add_day_of_month(self._cube, 'time') - if 'day_of_year' not in coords: - iris.coord_categorisation.add_day_of_year(self._cube, 'time') - if 'month_number' not in coords: - iris.coord_categorisation.add_month_number(self._cube, 'time') - if 'year' not in coords: - iris.coord_categorisation.add_year(self._cube, 'time') + if 'time' in coords: + if 'day_of_month' not in coords: + iris.coord_categorisation.add_day_of_month(self._cube, 'time') + if 'day_of_year' not in coords: + iris.coord_categorisation.add_day_of_year(self._cube, 'time') + if 'month_number' not in coords: + iris.coord_categorisation.add_month_number(self._cube, 'time') + if 'year' not in coords: + iris.coord_categorisation.add_year(self._cube, 'time') def _get_cmor_checker(table, From 10ff62ccf4e81ba150130ef43d9d833daef2ee0b Mon Sep 17 00:00:00 2001 From: Javier Vegas-Regidor Date: Tue, 3 Dec 2019 15:59:54 +0100 Subject: [PATCH 37/59] Clean a bit --- esmvalcore/_main.py | 2 +- esmvalcore/cmor/check.py | 30 +++++++++++++++--------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/esmvalcore/_main.py b/esmvalcore/_main.py index a2339c7ce7..1fbc396acf 100755 --- a/esmvalcore/_main.py +++ b/esmvalcore/_main.py @@ -161,7 +161,7 @@ def main(args): pattern if TASKSEP in pattern else pattern + TASKSEP + '*' for pattern in args.diagnostics or () } - cfg['check_level'] = args.check_level + cfg['check_level'] = CheckLevels[args.check_level.upper()] cfg['synda_download'] = args.synda_download for limit in ('max_datasets', 'max_years'): value = getattr(args, limit) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index bf5b3a273e..f454d93e5e 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -636,16 +636,13 @@ def report(self, level, message, *args): message """ msg = message.format(*args) - actual_level = self._check_level - if isinstance(actual_level, str): - actual_level = getattr(CheckLevels, self._check_level) if (level == CheckLevels.DEBUG or - actual_level == CheckLevels.IGNORE_ALL): + self._check_level == CheckLevels.IGNORE_ALL): if self._failerr: self._logger.debug(msg) else: self._debug_messages.append(msg) - elif level < actual_level: + elif level < self._check_level: if self._failerr: self._logger.warning(msg) else: @@ -711,16 +708,19 @@ def report_debug_message(self, message, *args): self.report(CheckLevels.DEBUG, message, *args) def _add_auxiliary_time_coordinates(self): - coords = [coord.name() for coord in self._cube.aux_coords] - if 'time' in coords: - if 'day_of_month' not in coords: - iris.coord_categorisation.add_day_of_month(self._cube, 'time') - if 'day_of_year' not in coords: - iris.coord_categorisation.add_day_of_year(self._cube, 'time') - if 'month_number' not in coords: - iris.coord_categorisation.add_month_number(self._cube, 'time') - if 'year' not in coords: - iris.coord_categorisation.add_year(self._cube, 'time') + try: + time = self._cube.coord('time') + except iris.exceptions.CoordinateNotFoundError: + return + + if not self._cube.coords('day_of_month'): + iris.coord_categorisation.add_day_of_month(self._cube, time) + if not self._cube.coords('day_of_year'): + iris.coord_categorisation.add_day_of_year(self._cube, time) + if not self._cube.coords('month_number'): + iris.coord_categorisation.add_month_number(self._cube, time) + if not self._cube.coords('year'): + iris.coord_categorisation.add_year(self._cube, time) def _get_cmor_checker(table, From d7d05616c08a03aec6fb2935358e4be0a353984c Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Tue, 3 Dec 2019 15:55:20 +0000 Subject: [PATCH 38/59] added documentation on check levels --- doc/esmvalcore/fixing_data.rst | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/doc/esmvalcore/fixing_data.rst b/doc/esmvalcore/fixing_data.rst index 3448c16ff1..8805005eca 100644 --- a/doc/esmvalcore/fixing_data.rst +++ b/doc/esmvalcore/fixing_data.rst @@ -245,3 +245,21 @@ missing coordinate you can create a fix for this model: data_cube = cubes.extract_strict('VAR_NAME') data_cube.add_aux_coord(coord, DIMENSIONS_INDEX_TUPLE) return [data_cube] + + +Running the checker in different modes +====================================== + +Sometimes it is inconvenient to run with full CMOR checks on, since one may just want to +run with a number of datasets that, albeit these having different issues, they need to be run as they +are and the user is interested in the output regardless of CMOR irregularities. For this purpose +there is an optional command line option `--check-level` that can take a number of values, listed +below from the lowest level of strictness to the highest: + +- ``IGNORE_ALL``: all issues will be reported as debug messages only (in the debug logs, code will continue running, + this is the lowest level of strictness); +- ``NEVER_FAIL``: all errors will be reported as warnings only (on screen and in the regular logs, code will continue running); +- ``CRITICAL``: only fail if there are critical errors (warnings reported on screen and in regular logs, code will continue running, unless + a critical failure is encountered); +- ``ERROR``: fail if there are any errors (DEFAULT); here errors are considered all CMOR check errors; this is the default behavior; +- ``WARNING``: fail if there are any warnings, this is the highest level of strictness. From 9cbd895238f001d7fd76e8f99c54e8d4daf75bc2 Mon Sep 17 00:00:00 2001 From: Valeriu Predoi Date: Tue, 3 Dec 2019 16:10:12 +0000 Subject: [PATCH 39/59] added more meat to the check levels doc --- doc/esmvalcore/fixing_data.rst | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/doc/esmvalcore/fixing_data.rst b/doc/esmvalcore/fixing_data.rst index 8805005eca..a91a9a8777 100644 --- a/doc/esmvalcore/fixing_data.rst +++ b/doc/esmvalcore/fixing_data.rst @@ -257,9 +257,13 @@ there is an optional command line option `--check-level` that can take a number below from the lowest level of strictness to the highest: - ``IGNORE_ALL``: all issues will be reported as debug messages only (in the debug logs, code will continue running, - this is the lowest level of strictness); -- ``NEVER_FAIL``: all errors will be reported as warnings only (on screen and in the regular logs, code will continue running); + this is the lowest level of strictness); use this at your own risk and be advised that any preprocessor or diagnostic + failures due to data irregulariries can only be corrected for if the ``main_log_debug.txt`` is examined; +- ``NEVER_FAIL``: all errors will be reported as warnings only (on screen and in the regular logs, code will continue running); use + this when you are willing to run with data that may have issues but you will monitor these issues via the warnings and + will contribute towards fixing whatever issue; - ``CRITICAL``: only fail if there are critical errors (warnings reported on screen and in regular logs, code will continue running, unless - a critical failure is encountered); + a critical failure is encountered); this is a very useful case for identifying serious issues with the data and can be a + powerful tool to produce data fixes with a sense of priority ie it helps the user to prioritize what fixes they produce; - ``ERROR``: fail if there are any errors (DEFAULT); here errors are considered all CMOR check errors; this is the default behavior; - ``WARNING``: fail if there are any warnings, this is the highest level of strictness. From b9375aab7a4b8aec86345f653a7a8943e1178e26 Mon Sep 17 00:00:00 2001 From: Javier Vegas-Regidor Date: Tue, 3 Dec 2019 18:20:28 +0100 Subject: [PATCH 40/59] Doc update --- doc/esmvalcore/fixing_data.rst | 63 +++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 20 deletions(-) diff --git a/doc/esmvalcore/fixing_data.rst b/doc/esmvalcore/fixing_data.rst index a91a9a8777..a82bc79a45 100644 --- a/doc/esmvalcore/fixing_data.rst +++ b/doc/esmvalcore/fixing_data.rst @@ -247,23 +247,46 @@ missing coordinate you can create a fix for this model: return [data_cube] -Running the checker in different modes -====================================== - -Sometimes it is inconvenient to run with full CMOR checks on, since one may just want to -run with a number of datasets that, albeit these having different issues, they need to be run as they -are and the user is interested in the output regardless of CMOR irregularities. For this purpose -there is an optional command line option `--check-level` that can take a number of values, listed -below from the lowest level of strictness to the highest: - -- ``IGNORE_ALL``: all issues will be reported as debug messages only (in the debug logs, code will continue running, - this is the lowest level of strictness); use this at your own risk and be advised that any preprocessor or diagnostic - failures due to data irregulariries can only be corrected for if the ``main_log_debug.txt`` is examined; -- ``NEVER_FAIL``: all errors will be reported as warnings only (on screen and in the regular logs, code will continue running); use - this when you are willing to run with data that may have issues but you will monitor these issues via the warnings and - will contribute towards fixing whatever issue; -- ``CRITICAL``: only fail if there are critical errors (warnings reported on screen and in regular logs, code will continue running, unless - a critical failure is encountered); this is a very useful case for identifying serious issues with the data and can be a - powerful tool to produce data fixes with a sense of priority ie it helps the user to prioritize what fixes they produce; -- ``ERROR``: fail if there are any errors (DEFAULT); here errors are considered all CMOR check errors; this is the default behavior; -- ``WARNING``: fail if there are any warnings, this is the highest level of strictness. +Customizing checker strictness +============================== + +The data checker classifies its issues using four different levels of +severity. From highest to lowest: + + - ``CRITICAL``: issues that most of the time will have severe consequences + + - ``ERROR``: issues that usually lead to unexpected errors, but can be safely + ignored sometimes + + - ``WARNING``: something is not up to the standard but is unlikely to have + consequences later + + - ``DEBUG``: any info that the checker wants to communicate. Regardless of + checker strictness, thos will always be reported as debug messages + +Users can have control about which levels of issues are interpreted as errors, +and therefore make the checker fail or warnings or debug messages. +For this purpose there is an optional command line option `--check-level` +that can take a number of values, listed below from the lowest level of +strictness to the highest: + +- ``IGNORE_ALL``: all issues, regardless of severity, will be reported as debug + messages only. Checker will never fail. Use this at your own risk and be + advised that any preprocessor or diagnostic failures due to data + irregulariries can only be corrected for if the ``main_log_debug.txt`` + is examined. + +- ``NEVER_FAIL``: all issues, regardless of severity, will be reported as + warnings. Use this at your own risk + +- ``CRITICAL``: only CRITICAL issues are treated as errors. We recommend not to + relay on this mode, although it can be useful if there an errors preventing + the run that you are sure you can manage on the diagnostics or that will + not affect you + +- ``ERROR``: fail if there are any CRITICAL or ERROR issues (DEFAULT); Provides + a good measure of safety + +- ``WARNING``: fail if there are any warnings, this is the highest level of + strictness. Mostly useful for checking datasets that you have produced, to + be sure that future users will not be distracted by inoffensive warnings. From dfb1e34436e6ce2e2710fac8f079c82aca0dddba Mon Sep 17 00:00:00 2001 From: sloosvel Date: Mon, 27 Jan 2020 09:23:41 +0100 Subject: [PATCH 41/59] Rename check levels, keep only one ignore option --- esmvalcore/_main.py | 15 ++-- esmvalcore/cmor/check.py | 15 ++-- esmvalcore/cmor/fix.py | 4 +- run.py | 3 + tests/unit/cmor/test_cmor_check.py | 113 ++++++++++++++--------------- tests/unit/cmor/test_fix.py | 4 +- 6 files changed, 75 insertions(+), 79 deletions(-) create mode 100644 run.py diff --git a/esmvalcore/_main.py b/esmvalcore/_main.py index 1fbc396acf..706077e29f 100755 --- a/esmvalcore/_main.py +++ b/esmvalcore/_main.py @@ -102,18 +102,19 @@ def get_args(): parser.add_argument( '--check-level', type=str, - choices=[val.name for val in CheckLevels if val != CheckLevels.DEBUG], - default='ERROR', + choices=[ + val.name.lower() for val in CheckLevels if val != CheckLevels.DEBUG + ], + default='default', help=""" Configure the severity of the errors that will make the CMOR check fail. Optional: true; Possible values: - IGNORE_ALL: all issues will be reported as debug messages - NEVER_FAIL: all errors will be reported as warnings - CRITICAL: only fail if there are critical errors - ERROR: fail if there are any errors (DEFAULT) - WARNING: fail if there are any warnings + ignore: all errors will be reported as warnings + relaxed: only fail if there are critical errors + default: fail if there are any errors + strict: fail if there are any warnings """ ) args = parser.parse_args() diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index f454d93e5e..7d4f2c8705 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -12,7 +12,7 @@ from .table import CMOR_TABLES CheckLevels = IntEnum( - 'CheckLevels', 'DEBUG WARNING ERROR CRITICAL NEVER_FAIL IGNORE_ALL') + 'CheckLevels', 'DEBUG STRICT DEFAULT RELAXED IGNORE') class CMORCheckError(Exception): @@ -61,7 +61,7 @@ def __init__(self, var_info, frequency=None, fail_on_error=False, - check_level=CheckLevels.ERROR, + check_level=CheckLevels.DEFAULT, automatic_fixes=False): self._cube = cube @@ -636,8 +636,7 @@ def report(self, level, message, *args): message """ msg = message.format(*args) - if (level == CheckLevels.DEBUG or - self._check_level == CheckLevels.IGNORE_ALL): + if level == CheckLevels.DEBUG: if self._failerr: self._logger.debug(msg) else: @@ -666,7 +665,7 @@ def report_critical(self, message, *args): arguments to format the message string. """ - self.report(CheckLevels.CRITICAL, message, *args) + self.report(CheckLevels.RELAXED, message, *args) def report_error(self, message, *args): """Report a normal error. @@ -679,7 +678,7 @@ def report_error(self, message, *args): arguments to format the message string. """ - self.report(CheckLevels.ERROR, message, *args) + self.report(CheckLevels.DEFAULT, message, *args) def report_warning(self, message, *args): """Report a warning level error. @@ -692,7 +691,7 @@ def report_warning(self, message, *args): arguments to format the message string. """ - self.report(CheckLevels.WARNING, message, *args) + self.report(CheckLevels.STRICT, message, *args) def report_debug_message(self, message, *args): """Report a debug message. @@ -728,7 +727,7 @@ def _get_cmor_checker(table, short_name, frequency, fail_on_error=False, - check_level=CheckLevels.ERROR, + check_level=CheckLevels.DEFAULT, automatic_fixes=False): """Get a CMOR checker/fixer.""" if table not in CMOR_TABLES: diff --git a/esmvalcore/cmor/fix.py b/esmvalcore/cmor/fix.py index 0ac1d683f6..1c6e7dd95f 100644 --- a/esmvalcore/cmor/fix.py +++ b/esmvalcore/cmor/fix.py @@ -57,7 +57,7 @@ def fix_metadata(cubes, cmor_table=None, mip=None, frequency=None, - check_level=CheckLevels.ERROR): + check_level=CheckLevels.DEFAULT): """ Fix cube metadata if fixes are required and check it anyway. @@ -161,7 +161,7 @@ def fix_data(cube, cmor_table=None, mip=None, frequency=None, - check_level=CheckLevels.ERROR): + check_level=CheckLevels.DEFAULT): """ Fix cube data if fixes add present and check it anyway. diff --git a/run.py b/run.py new file mode 100644 index 0000000000..e2ad747d51 --- /dev/null +++ b/run.py @@ -0,0 +1,3 @@ +# import esmvalcore._main + +# esmvalcore._main.run() \ No newline at end of file diff --git a/tests/unit/cmor/test_cmor_check.py b/tests/unit/cmor/test_cmor_check.py index a139292ee7..0a379424c2 100644 --- a/tests/unit/cmor/test_cmor_check.py +++ b/tests/unit/cmor/test_cmor_check.py @@ -132,7 +132,7 @@ def test_check(self): self._check_cube() def _check_cube(self, automatic_fixes=False, frequency=None, - check_level=CheckLevels.ERROR): + check_level=CheckLevels.DEFAULT): """Apply checks and optionally automatic fixes to self.cube.""" def checker(cube): return CMORCheck( @@ -146,7 +146,7 @@ def checker(cube): self.cube = checker(self.cube).check_data() def _check_cube_metadata(self, automatic_fixes=False, frequency=None, - check_level=CheckLevels.ERROR): + check_level=CheckLevels.DEFAULT): """Apply checks and optionally automatic fixes to self.cube.""" def checker(cube): return CMORCheck( @@ -348,7 +348,7 @@ def test_check_bad_var_standard_name_relaxed_flag(self): self.cube = self.get_cube(self.var_info) self.cube.standard_name = 'wind_speed' self._check_warnings_on_metadata(automatic_fixes=False, - check_level=CheckLevels.CRITICAL) + check_level=CheckLevels.RELAXED) def test_check_bad_var_long_name_relaxed_flag(self): """Test check reports warning for a bad variable long_name with @@ -356,7 +356,7 @@ def test_check_bad_var_long_name_relaxed_flag(self): self.cube = self.get_cube(self.var_info) self.cube.long_name = "Near-Surface Wind Speed" self._check_warnings_on_metadata(automatic_fixes=False, - check_level=CheckLevels.CRITICAL) + check_level=CheckLevels.RELAXED) def test_check_bad_var_units_relaxed_flag(self): """Test check reports warning for a bad variable units with @@ -364,7 +364,7 @@ def test_check_bad_var_units_relaxed_flag(self): self.cube = self.get_cube(self.var_info) self.cube.units = "kg" self._check_warnings_on_metadata(automatic_fixes=False, - check_level=CheckLevels.CRITICAL) + check_level=CheckLevels.RELAXED) def test_check_bad_attributes_relaxed_flag(self): """Test check report warnings for a bad variable attribute with @@ -374,7 +374,7 @@ def test_check_bad_attributes_relaxed_flag(self): self.cube = self.get_cube(self.var_info) self.cube.attributes['positive'] = "Wrong attribute" self._check_warnings_on_metadata(automatic_fixes=False, - check_level=CheckLevels.CRITICAL) + check_level=CheckLevels.RELAXED) def test_check_bad_rank_relaxed_flag(self): """Test check report warnings for a bad variable rank with @@ -383,7 +383,7 @@ def test_check_bad_rank_relaxed_flag(self): self.cube.remove_coord('latitude') self.cube.add_aux_coord(lat, self.cube.coord_dims('longitude')) self._check_warnings_on_metadata(automatic_fixes=False, - check_level=CheckLevels.CRITICAL) + check_level=CheckLevels.RELAXED) def test_check_bad_coord_standard_name_relaxed_flag(self): """Test check reports warning for bad coord var_name with @@ -391,28 +391,28 @@ def test_check_bad_coord_standard_name_relaxed_flag(self): self.var_info.table_type = 'CMIP5' self.cube.coord('longitude').var_name = 'bad_name' self._check_warnings_on_metadata(automatic_fixes=False, - check_level=CheckLevels.CRITICAL) + check_level=CheckLevels.RELAXED) def test_check_missing_lon_relaxed_flag(self): """Test check fails for missing longitude with --cmor-check relaxed""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('longitude') self._check_fails_in_metadata(automatic_fixes=False, - check_level=CheckLevels.CRITICAL) + check_level=CheckLevels.RELAXED) def test_check_missing_lat_relaxed_flag(self): """Test check fails for missing latitude with --cmor-check relaxed""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('latitude') self._check_fails_in_metadata(automatic_fixes=False, - check_level=CheckLevels.CRITICAL) + check_level=CheckLevels.RELAXED) def test_check_missing_time_relaxed_flag(self): """Test check fails for missing latitude with --cmor-check relaxed""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('time') self._check_fails_in_metadata(automatic_fixes=False, - check_level=CheckLevels.CRITICAL) + check_level=CheckLevels.RELAXED) def test_check_missing_coord_relaxed_flag(self): """Test check reports warning for missing coord other than lat and lon @@ -421,91 +421,94 @@ def test_check_missing_coord_relaxed_flag(self): self.var_info.coordinates = {'height2m': CoordinateInfoMock('height2m') } self._check_warnings_on_metadata(automatic_fixes=False, - check_level=CheckLevels.CRITICAL) + check_level=CheckLevels.RELAXED) def test_check_bad_var_standard_name_none_flag(self): - """Test check does not report for a bad variable standard_name with - --cmor-check none.""" + """Test check reports warning for a bad variable standard_name with + --cmor-check ignore.""" self.cube = self.get_cube(self.var_info) self.cube.standard_name = 'wind_speed' - self._check_no_reports_on_metadata(automatic_fixes=False, - check_level=CheckLevels.IGNORE_ALL) + self._check_warnings_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE) def test_check_bad_var_long_name_none_flag(self): - """Test check does not report for a bad variable long_name with - --cmor-check none.""" + """Test check reports warning for a bad variable long_name with + --cmor-check ignore.""" self.cube = self.get_cube(self.var_info) self.cube.long_name = "Near-Surface Wind Speed" - self._check_no_reports_on_metadata(automatic_fixes=False, - check_level=CheckLevels.IGNORE_ALL) + self._check_warnings_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE) def test_check_bad_var_units_none_flag(self): - """Test check does not report for a bad variable unit with - --cmor-check none.""" + """Test check reports warning for a bad variable unit with + --cmor-check ignore.""" self.cube = self.get_cube(self.var_info) self.cube.units = "kg" - self._check_no_reports_on_metadata(automatic_fixes=False, - check_level=CheckLevels.IGNORE_ALL) + self._check_warnings_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE) def test_check_bad_attributes_none_flag(self): - """Test check does not report for a bad variable attribute with - --cmor-check none.""" + """Test check reports warning for a bad variable attribute with + --cmor-check ignore.""" self.var_info.standard_name = "surface_upward_latent_heat_flux" self.var_info.positive = "up" self.cube = self.get_cube(self.var_info) self.cube.attributes['positive'] = "Wrong attribute" - self._check_no_reports_on_metadata(automatic_fixes=False, - check_level=CheckLevels.IGNORE_ALL) + self._check_warnings_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE) def test_check_bad_rank_none_flag(self): - """Test check does not report for a bad variable rank with - --cmor-check relaxed.""" + """Test check reports warning for a bad variable rank with + --cmor-check ignore.""" lat = iris.coords.AuxCoord.from_coord(self.cube.coord('latitude')) self.cube.remove_coord('latitude') self.cube.add_aux_coord(lat, self.cube.coord_dims('longitude')) self._check_warnings_on_metadata(automatic_fixes=False, - check_level=CheckLevels.CRITICAL) + check_level=CheckLevels.IGNORE) def test_check_bad_coord_standard_name_none_flag(self): - """Test check reports nothing for bad coord var_name with - --cmor-check none.""" + """Test check reports warning for bad coord var_name with + --cmor-check ignore.""" self.var_info.table_type = 'CMIP5' self.cube.coord('longitude').var_name = 'bad_name' - self._check_no_reports_on_metadata(automatic_fixes=False, - check_level=CheckLevels.IGNORE_ALL) + self._check_warnings_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE) def test_check_missing_lon_none_flag(self): - """Test check fails for missing longitude with --cmor-check none""" + """Test check reports warning for missing longitude with + --cmor-check ignore""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('longitude') - self._check_no_reports_on_metadata(automatic_fixes=False, - check_level=CheckLevels.IGNORE_ALL) + self._check_warnings_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE) def test_check_missing_lat_none_flag(self): - """Test check fails for missing latitude with --cmor-check none""" + """Test check reports warning for missing latitude with + --cmor-check ignore""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('latitude') - self._check_no_reports_on_metadata(automatic_fixes=False, - check_level=CheckLevels.IGNORE_ALL) + self._check_warnings_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE) def test_check_missing_time_none_flag(self): - """Test check fails for missing time with --cmor-check none""" + """Test check reports warning for missing time + with --cmor-check ignore""" self.var_info.table_type = 'CMIP5' self.cube.remove_coord('time') - self._check_no_reports_on_metadata(automatic_fixes=False, - check_level=CheckLevels.IGNORE_ALL) + self._check_warnings_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE) def test_check_missing_coord_none_flag(self): - """Test check reports nothing for missing coord other than lat, lon and - time with --cmor-check none""" + """Test check reports warning for missing coord other than lat, lon and + time with --cmor-check ignore""" self.cube = self.get_cube(self.var_info) self.var_info.coordinates = {'height2m': CoordinateInfoMock('height2m') } - self._check_no_reports_on_metadata(automatic_fixes=False, - check_level=CheckLevels.IGNORE_ALL) + self._check_warnings_on_metadata(automatic_fixes=False, + check_level=CheckLevels.IGNORE) def _check_fails_in_metadata(self, automatic_fixes=False, frequency=None, - check_level=CheckLevels.ERROR): + check_level=CheckLevels.DEFAULT): checker = CMORCheck( self.cube, self.var_info, @@ -516,7 +519,7 @@ def _check_fails_in_metadata(self, automatic_fixes=False, frequency=None, checker.check_metadata() def _check_warnings_on_metadata(self, automatic_fixes=False, - check_level=CheckLevels.ERROR): + check_level=CheckLevels.DEFAULT): checker = CMORCheck( self.cube, self.var_info, automatic_fixes=automatic_fixes, check_level=check_level @@ -524,16 +527,6 @@ def _check_warnings_on_metadata(self, automatic_fixes=False, checker.check_metadata() self.assertTrue(checker.has_warnings()) - def _check_no_reports_on_metadata(self, automatic_fixes=False, - check_level=CheckLevels.ERROR): - checker = CMORCheck( - self.cube, self.var_info, automatic_fixes=automatic_fixes, - check_level=check_level - ) - checker.check_metadata() - self.assertFalse(checker.has_errors()) - self.assertFalse(checker.has_warnings()) - def _check_debug_messages_on_metadata(self): checker = CMORCheck(self.cube, self.var_info) checker.check_metadata() diff --git a/tests/unit/cmor/test_fix.py b/tests/unit/cmor/test_fix.py index c9eed61c48..ad5de30970 100644 --- a/tests/unit/cmor/test_fix.py +++ b/tests/unit/cmor/test_fix.py @@ -151,7 +151,7 @@ def test_cmor_checker_called(self): mip='mip', short_name='short_name', table='cmor_table', - check_level=CheckLevels.ERROR,) + check_level=CheckLevels.DEFAULT,) checker.assert_called_once_with(self.cube) checker.return_value.check_metadata.assert_called_once_with() @@ -198,7 +198,7 @@ def test_cmor_checker_called(self): 'cmor_table', 'mip', 'frequency') get_mock.assert_called_once_with( automatic_fixes=True, - check_level=CheckLevels.ERROR, + check_level=CheckLevels.DEFAULT, fail_on_error=False, frequency='frequency', mip='mip', From 22379415326e373b01fe8ed382df9830b3286d1d Mon Sep 17 00:00:00 2001 From: sloosvel Date: Mon, 27 Jan 2020 09:31:21 +0100 Subject: [PATCH 42/59] Remove debug file --- run.py | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 run.py diff --git a/run.py b/run.py deleted file mode 100644 index e2ad747d51..0000000000 --- a/run.py +++ /dev/null @@ -1,3 +0,0 @@ -# import esmvalcore._main - -# esmvalcore._main.run() \ No newline at end of file From 7f04bae9d488a0d700b13d587c05e54b08e480c3 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Mon, 27 Jan 2020 09:39:39 +0100 Subject: [PATCH 43/59] Update documentation with name changes --- doc/esmvalcore/fixing_data.rst | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/doc/esmvalcore/fixing_data.rst b/doc/esmvalcore/fixing_data.rst index a82bc79a45..a28ddd316b 100644 --- a/doc/esmvalcore/fixing_data.rst +++ b/doc/esmvalcore/fixing_data.rst @@ -253,16 +253,16 @@ Customizing checker strictness The data checker classifies its issues using four different levels of severity. From highest to lowest: - - ``CRITICAL``: issues that most of the time will have severe consequences + - ``CRITICAL``: issues that most of the time will have severe consequences. - ``ERROR``: issues that usually lead to unexpected errors, but can be safely - ignored sometimes + ignored sometimes. - ``WARNING``: something is not up to the standard but is unlikely to have - consequences later + consequences later. - ``DEBUG``: any info that the checker wants to communicate. Regardless of - checker strictness, thos will always be reported as debug messages + checker strictness, those will always be reported as debug messages. Users can have control about which levels of issues are interpreted as errors, and therefore make the checker fail or warnings or debug messages. @@ -270,23 +270,17 @@ For this purpose there is an optional command line option `--check-level` that can take a number of values, listed below from the lowest level of strictness to the highest: -- ``IGNORE_ALL``: all issues, regardless of severity, will be reported as debug - messages only. Checker will never fail. Use this at your own risk and be - advised that any preprocessor or diagnostic failures due to data - irregulariries can only be corrected for if the ``main_log_debug.txt`` - is examined. +- ``ignore``: all issues, regardless of severity, will be reported as + warnings. Checker will never fail. Use this at your own risk. -- ``NEVER_FAIL``: all issues, regardless of severity, will be reported as - warnings. Use this at your own risk - -- ``CRITICAL``: only CRITICAL issues are treated as errors. We recommend not to - relay on this mode, although it can be useful if there an errors preventing +- ``relaxed``: only CRITICAL issues are treated as errors. We recommend not to + relay on this mode, although it can be useful if there are errors preventing the run that you are sure you can manage on the diagnostics or that will - not affect you + not affect you. -- ``ERROR``: fail if there are any CRITICAL or ERROR issues (DEFAULT); Provides - a good measure of safety +- ``default``: fail if there are any CRITICAL or ERROR issues (DEFAULT); Provides + a good measure of safety. -- ``WARNING``: fail if there are any warnings, this is the highest level of +- ``strict``: fail if there are any warnings, this is the highest level of strictness. Mostly useful for checking datasets that you have produced, to be sure that future users will not be distracted by inoffensive warnings. From 8c8b05cebe7e542f192c27baca4138c2b8861f53 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Fri, 31 Jan 2020 16:01:39 +0100 Subject: [PATCH 44/59] Report warnings when converting units --- esmvalcore/cmor/check.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 7d4f2c8705..cadf3206b0 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -242,12 +242,17 @@ def _check_var_metadata(self): if self._cmor_var.units: units = self._get_effective_units() - - if not self._cube.units.is_convertible(units): - self.report_error( - f'Variable {self._cube.var_name} units ' - f'{self._cube.units} can not be ' - f'converted to {self._cmor_var.units}') + if self._cube.units != units: + if not self._cube.units.is_convertible(units): + self.report_error( + f'Variable {self._cube.var_name} units ' + f'{self._cube.units} can not be ' + f'converted to {self._cmor_var.units}') + else: + self.report_warning( + f'Variable {self._cube.var_name} units ' + f'{self._cube.units} will be ' + f'converted to {self._cmor_var.units}') # Check other variable attributes that match entries in cube.attributes attrs = ('positive', ) @@ -383,10 +388,15 @@ def _check_coord(self, cmor, coord, var_name): fixed = False if self.automatic_fixes: try: + old_unit = coord.units new_unit = cf_units.Unit(cmor.units, coord.units.calendar) coord.convert_units(new_unit) fixed = True + self.report_warning( + f'Coordinate {coord.var_name} units ' + f'{str(old_unit)} ' + f'converted to {cmor.units}') except ValueError: pass if not fixed: From df2c681e4f543b252010a91f5c36955b6c9ab970 Mon Sep 17 00:00:00 2001 From: sloosvel <45196700+sloosvel@users.noreply.github.com> Date: Fri, 31 Jan 2020 16:01:44 +0100 Subject: [PATCH 45/59] Update doc/esmvalcore/fixing_data.rst Co-Authored-By: Bouwe Andela --- doc/esmvalcore/fixing_data.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/esmvalcore/fixing_data.rst b/doc/esmvalcore/fixing_data.rst index a28ddd316b..058c3b3054 100644 --- a/doc/esmvalcore/fixing_data.rst +++ b/doc/esmvalcore/fixing_data.rst @@ -274,7 +274,7 @@ strictness to the highest: warnings. Checker will never fail. Use this at your own risk. - ``relaxed``: only CRITICAL issues are treated as errors. We recommend not to - relay on this mode, although it can be useful if there are errors preventing + rely on this mode, although it can be useful if there are errors preventing the run that you are sure you can manage on the diagnostics or that will not affect you. From e27617c5209f669a5f36d1d94f27870a26ff430e Mon Sep 17 00:00:00 2001 From: sloosvel Date: Wed, 19 Feb 2020 12:58:50 +0100 Subject: [PATCH 46/59] Update docstrings --- esmvalcore/cmor/check.py | 27 +++++++++++++++++++++++++++ esmvalcore/cmor/fix.py | 4 ++++ 2 files changed, 31 insertions(+) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 5a492e73fc..f123abd66d 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -13,6 +13,16 @@ CheckLevels = IntEnum( 'CheckLevels', 'DEBUG STRICT DEFAULT RELAXED IGNORE') +"""Level of strictness of the checks. + + Values + ------ + - DEBUG: Report any debug message that the checker wants to communicate. + - STRICT: Fail if there are warnings regarding compliance of CMOR standards. + - DEFAULT: Fail if cubes present any discrepancy with CMOR standards. + - RELAXED: Fail if cubes present severe discrepancies with CMOR standards. + - IGNORE: Do not fail for any discrepancy with CMOR standards. +""" class CMORCheckError(Exception): @@ -39,6 +49,8 @@ class CMORCheck(): automatic_fixes: bool If True, CMORCheck will try to apply automatic fixes for any detected error, if possible. + check_level: enum.IntEnum + Level of strictness of the checks. Attributes ---------- @@ -89,6 +101,11 @@ def check_metadata(self, logger=None): - Equivalent calendars will all default to the same name. - Time units will be set to days since 1850-01-01 + + Parameters + ---------- + logger + Raises ------ CMORCheckError @@ -124,6 +141,10 @@ def check_data(self, logger=None): It will also report some warnings in case of minor errors. + Parameters + ---------- + logger + Raises ------ CMORCheckError @@ -763,6 +784,8 @@ def cmor_check_metadata(cube, cmor_table, mip, Variable's short name. frequency: basestring Data frequency. + check_level: enum.IntEnum + Level of strictness of the checks. """ checker = _get_cmor_checker(cmor_table, mip, @@ -789,6 +812,8 @@ def cmor_check_data(cube, cmor_table, mip, short_name, frequency, check_level): Variable's short name frequency: basestring Data frequency + check_level: enum.IntEnum + Level of strictness of the checks. """ checker = _get_cmor_checker(cmor_table, mip, short_name, frequency, @@ -815,6 +840,8 @@ def cmor_check(cube, cmor_table, mip, short_name, frequency, check_level): Variable's short name. frequency: basestring Data frequency. + check_level: enum.IntEnum + Level of strictness of the checks. """ cmor_check_metadata(cube, cmor_table, mip, short_name, frequency, diff --git a/esmvalcore/cmor/fix.py b/esmvalcore/cmor/fix.py index 1c6e7dd95f..0ef48480a4 100644 --- a/esmvalcore/cmor/fix.py +++ b/esmvalcore/cmor/fix.py @@ -84,6 +84,8 @@ def fix_metadata(cubes, frequency: str, optional Variable's data frequency, if available + check_level: enum.IntEnum + Level of strictness of the checks. Set to default. Returns ------- @@ -190,6 +192,8 @@ def fix_data(cube, frequency: str, optional Variable's data frequency, if available + check_level: enum.IntEnum + Level of strictness of the checks. Set to default. Returns ------- From 1f45441b9898b5b8d490170c0e0d4ae6b0a108d5 Mon Sep 17 00:00:00 2001 From: sloosvel <45196700+sloosvel@users.noreply.github.com> Date: Thu, 27 Feb 2020 12:36:09 +0100 Subject: [PATCH 47/59] Update esmvalcore/cmor/check.py Co-Authored-By: Bouwe Andela --- esmvalcore/cmor/check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 90ae236eda..d379020d58 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -15,7 +15,7 @@ 'CheckLevels', 'DEBUG STRICT DEFAULT RELAXED IGNORE') """Level of strictness of the checks. - Values + Attributes ------ - DEBUG: Report any debug message that the checker wants to communicate. - STRICT: Fail if there are warnings regarding compliance of CMOR standards. From 24c4cad508a5b5b9b4778bb7112b341fc6d4909a Mon Sep 17 00:00:00 2001 From: sloosvel <45196700+sloosvel@users.noreply.github.com> Date: Thu, 27 Feb 2020 12:36:56 +0100 Subject: [PATCH 48/59] Update esmvalcore/cmor/check.py Co-Authored-By: Bouwe Andela --- esmvalcore/cmor/check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index d379020d58..c308d7670e 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -49,7 +49,7 @@ class CMORCheck(): automatic_fixes: bool If True, CMORCheck will try to apply automatic fixes for any detected error, if possible. - check_level: enum.IntEnum + check_level: CheckLevels Level of strictness of the checks. Attributes From c0f4556aada61769c70070ea969922c476ba577f Mon Sep 17 00:00:00 2001 From: sloosvel <45196700+sloosvel@users.noreply.github.com> Date: Thu, 27 Feb 2020 12:37:21 +0100 Subject: [PATCH 49/59] Update esmvalcore/cmor/check.py Co-Authored-By: Bouwe Andela --- esmvalcore/cmor/check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index c308d7670e..ad1383b2d7 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -828,7 +828,7 @@ def cmor_check_metadata(cube, cmor_table, mip, Variable's short name. frequency: basestring Data frequency. - check_level: enum.IntEnum + check_level: CheckLevels Level of strictness of the checks. """ From c3315a897bb0e3ed1cdf741316cc9b879c6f6a61 Mon Sep 17 00:00:00 2001 From: sloosvel <45196700+sloosvel@users.noreply.github.com> Date: Thu, 27 Feb 2020 12:38:34 +0100 Subject: [PATCH 50/59] Update esmvalcore/cmor/check.py Co-Authored-By: Bouwe Andela --- esmvalcore/cmor/check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index ad1383b2d7..5c875b9f3e 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -856,7 +856,7 @@ def cmor_check_data(cube, cmor_table, mip, short_name, frequency, check_level): Variable's short name frequency: basestring Data frequency - check_level: enum.IntEnum + check_level: CheckLevels Level of strictness of the checks. """ From 913e9b0a67e1acd49dcfcd6e902bfbca5de06bbf Mon Sep 17 00:00:00 2001 From: sloosvel <45196700+sloosvel@users.noreply.github.com> Date: Thu, 27 Feb 2020 12:39:52 +0100 Subject: [PATCH 51/59] Update esmvalcore/cmor/fix.py Co-Authored-By: Bouwe Andela --- esmvalcore/cmor/fix.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvalcore/cmor/fix.py b/esmvalcore/cmor/fix.py index 1bfb72ba47..391c186a5e 100644 --- a/esmvalcore/cmor/fix.py +++ b/esmvalcore/cmor/fix.py @@ -80,7 +80,7 @@ def fix_metadata(cubes, frequency: str, optional Variable's data frequency, if available - check_level: enum.IntEnum + check_level: CheckLevels Level of strictness of the checks. Set to default. Returns From 8f83249c51456b81efab8422345cf717a301aa38 Mon Sep 17 00:00:00 2001 From: sloosvel <45196700+sloosvel@users.noreply.github.com> Date: Thu, 27 Feb 2020 12:40:31 +0100 Subject: [PATCH 52/59] Update esmvalcore/cmor/fix.py Co-Authored-By: Bouwe Andela --- esmvalcore/cmor/fix.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvalcore/cmor/fix.py b/esmvalcore/cmor/fix.py index 391c186a5e..c660b6905d 100644 --- a/esmvalcore/cmor/fix.py +++ b/esmvalcore/cmor/fix.py @@ -179,7 +179,7 @@ def fix_data(cube, Variable's MIP frequency: str, optional Variable's data frequency, if available - check_level: enum.IntEnum + check_level: CheckLevels Level of strictness of the checks. Set to default. Returns From 91a205e3aa671019616f931a6d58bc70f1e49fc8 Mon Sep 17 00:00:00 2001 From: sloosvel <45196700+sloosvel@users.noreply.github.com> Date: Thu, 27 Feb 2020 12:45:39 +0100 Subject: [PATCH 53/59] Update esmvalcore/cmor/check.py Co-Authored-By: Bouwe Andela --- esmvalcore/cmor/check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 5c875b9f3e..65b66c2bf4 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -811,7 +811,7 @@ def _checker(cube): def cmor_check_metadata(cube, cmor_table, mip, short_name, frequency, - check_level): + check_level=CheckLevels.DEFAULT): """Check if metadata conforms to variable's CMOR definiton. None of the checks at this step will force the cube to load the data. From 4e102d09b4be9ce491281fb91dcf3b54619747f8 Mon Sep 17 00:00:00 2001 From: sloosvel <45196700+sloosvel@users.noreply.github.com> Date: Thu, 27 Feb 2020 12:49:37 +0100 Subject: [PATCH 54/59] Apply suggestions from code review Co-Authored-By: Bouwe Andela --- esmvalcore/_recipe.py | 6 +++--- tests/integration/test_recipe.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/esmvalcore/_recipe.py b/esmvalcore/_recipe.py index 640335cc44..9fe543916e 100644 --- a/esmvalcore/_recipe.py +++ b/esmvalcore/_recipe.py @@ -299,7 +299,7 @@ def _get_default_settings(variable, config_user, derive=False): settings['fix_file']['output_dir'] = fix_dir # Cube fixes fix['frequency'] = variable['frequency'] - fix['check_level'] = config_user.get('check_level', None) + fix['check_level'] = config_user['check_level'] settings['fix_metadata'] = dict(fix) settings['fix_data'] = dict(fix) @@ -329,7 +329,7 @@ def _get_default_settings(variable, config_user, derive=False): 'mip': variable['mip'], 'short_name': variable['short_name'], 'frequency': variable['frequency'], - 'check_level': config_user.get('check_level', None) + 'check_level': config_user['check_level'] } # Configure final CMOR data check settings['cmor_check_data'] = { @@ -337,7 +337,7 @@ def _get_default_settings(variable, config_user, derive=False): 'mip': variable['mip'], 'short_name': variable['short_name'], 'frequency': variable['frequency'], - 'check_level': config_user.get('check_level', None) + 'check_level': config_user['check_level'] } # Clean up fixed files diff --git a/tests/integration/test_recipe.py b/tests/integration/test_recipe.py index 0b79c2909f..05981cce5c 100644 --- a/tests/integration/test_recipe.py +++ b/tests/integration/test_recipe.py @@ -352,7 +352,7 @@ def test_default_preprocessor(tmp_path, patched_datafinder, config_user): 'output_dir': fix_dir, }, 'fix_data': { - 'check_level': None, + 'check_level': CheckLevels.DEFAULT, 'project': 'CMIP5', 'dataset': 'CanESM2', 'short_name': 'chl', @@ -360,7 +360,7 @@ def test_default_preprocessor(tmp_path, patched_datafinder, config_user): 'frequency': 'yr', }, 'fix_metadata': { - 'check_level': None, + 'check_level': CheckLevels.DEFAULT, 'project': 'CMIP5', 'dataset': 'CanESM2', 'short_name': 'chl', @@ -376,7 +376,7 @@ def test_default_preprocessor(tmp_path, patched_datafinder, config_user): 'end_day': 1, }, 'cmor_check_metadata': { - 'check_level': None, + 'check_level': CheckLevels.DEFAULT, 'cmor_table': 'CMIP5', 'mip': 'Oyr', 'short_name': 'chl', From e0d9340686baa84bdd87be259856109125d2456d Mon Sep 17 00:00:00 2001 From: sloosvel Date: Thu, 27 Feb 2020 15:02:38 +0100 Subject: [PATCH 55/59] Improve warning message --- esmvalcore/cmor/fix.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esmvalcore/cmor/fix.py b/esmvalcore/cmor/fix.py index c660b6905d..a30be15cc2 100644 --- a/esmvalcore/cmor/fix.py +++ b/esmvalcore/cmor/fix.py @@ -140,8 +140,8 @@ def _get_single_cube(cube_list, short_name, project, dataset): 'Found variable %s in %s:%s, but there were other present in ' 'the file. Those extra variables are usually metadata ' '(cell area, latitude descriptions) that was not saved ' - 'properly. It is possible that errors appear further on ' - 'because of this. \nFull list of cubes encountered: %s', + 'according to CF-conventions. It is possible that errors appear ' + 'further on because of this. \nFull list of cubes encountered: %s', short_name, project, dataset, From 56ce2c2b8f397fb97ae669375c84272441561fe1 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Thu, 27 Feb 2020 15:04:25 +0100 Subject: [PATCH 56/59] Report missing coordinates with levels as critical --- esmvalcore/cmor/check.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 65b66c2bf4..84c8a365be 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -352,7 +352,8 @@ def _check_dim_names(self): ) except iris.exceptions.CoordinateNotFoundError: if coordinate.standard_name in ['time', 'latitude', - 'longitude']: + 'longitude'] \ + or coordinate.requested: self.report_critical( self._does_msg, coordinate.name, 'exist') else: From 1d5a0eb21a176b1c49705fc167e422bb73052b40 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Thu, 27 Feb 2020 15:12:30 +0100 Subject: [PATCH 57/59] Improve docstring --- esmvalcore/cmor/check.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 84c8a365be..c1e74974a9 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -100,7 +100,8 @@ def check_metadata(self, logger=None): Parameters ---------- - logger + logger: logging.Logger + Given logger. Raises ------ @@ -139,7 +140,8 @@ def check_data(self, logger=None): Parameters ---------- - logger + logger: logging.Logger + Given logger. Raises ------ @@ -183,7 +185,8 @@ def report_warnings(self): Parameters ---------- - logger + logger: logging.Logger + Given logger """ if self.has_warnings(): @@ -196,7 +199,8 @@ def report_debug_messages(self): Parameters ---------- - logger + logger: logging.Logger + Given logger. """ if self.has_debug_messages(): From 4c96139f5730e66ec51a9394706c8c4d63740c83 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Thu, 27 Feb 2020 15:46:15 +0100 Subject: [PATCH 58/59] Fix identation --- esmvalcore/cmor/check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index fdfce87cfd..61c4f6f6fd 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -356,8 +356,8 @@ def _check_dim_names(self): ) except iris.exceptions.CoordinateNotFoundError: if coordinate.standard_name in ['time', 'latitude', - 'longitude'] \ - or coordinate.requested: + 'longitude'] or \ + coordinate.requested: self.report_critical( self._does_msg, coordinate.name, 'exist') else: From 275d65cd72a51c940a1fdbf3637396f71d7d9f91 Mon Sep 17 00:00:00 2001 From: sloosvel Date: Thu, 27 Feb 2020 15:57:45 +0100 Subject: [PATCH 59/59] Fix tests --- tests/integration/test_recipe.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_recipe.py b/tests/integration/test_recipe.py index 05981cce5c..c9efc7a467 100644 --- a/tests/integration/test_recipe.py +++ b/tests/integration/test_recipe.py @@ -15,6 +15,8 @@ from esmvalcore._task import DiagnosticTask from esmvalcore.preprocessor import DEFAULT_ORDER, PreprocessingTask from esmvalcore.preprocessor._io import concatenate_callback +from esmvalcore.cmor.check import CheckLevels + from .test_diagnostic_run import write_config_user_file from .test_provenance import check_provenance @@ -65,6 +67,7 @@ def config_user(tmp_path): cfg = esmvalcore._config.read_config_user_file(filename, 'recipe_test') cfg['synda_download'] = False cfg['output_file_type'] = 'png' + cfg['check_level'] = CheckLevels.DEFAULT return cfg @@ -383,7 +386,7 @@ def test_default_preprocessor(tmp_path, patched_datafinder, config_user): 'frequency': 'yr', }, 'cmor_check_data': { - 'check_level': None, + 'check_level': CheckLevels.DEFAULT, 'cmor_table': 'CMIP5', 'mip': 'Oyr', 'short_name': 'chl', @@ -442,7 +445,7 @@ def test_default_fx_preprocessor(tmp_path, patched_datafinder, config_user): 'output_dir': fix_dir, }, 'fix_data': { - 'check_level': None, + 'check_level': CheckLevels.DEFAULT, 'project': 'CMIP5', 'dataset': 'CanESM2', 'short_name': 'sftlf', @@ -450,7 +453,7 @@ def test_default_fx_preprocessor(tmp_path, patched_datafinder, config_user): 'frequency': 'fx', }, 'fix_metadata': { - 'check_level': None, + 'check_level': CheckLevels.DEFAULT, 'project': 'CMIP5', 'dataset': 'CanESM2', 'short_name': 'sftlf', @@ -458,14 +461,14 @@ def test_default_fx_preprocessor(tmp_path, patched_datafinder, config_user): 'frequency': 'fx', }, 'cmor_check_metadata': { - 'check_level': None, + 'check_level': CheckLevels.DEFAULT, 'cmor_table': 'CMIP5', 'mip': 'fx', 'short_name': 'sftlf', 'frequency': 'fx', }, 'cmor_check_data': { - 'check_level': None, + 'check_level': CheckLevels.DEFAULT, 'cmor_table': 'CMIP5', 'mip': 'fx', 'short_name': 'sftlf',