From 4b8bf66ce6ef51cc6814b5d18357ae91684eb0b7 Mon Sep 17 00:00:00 2001 From: Manuel Schlund <32543114+schlunma@users.noreply.github.com> Date: Thu, 13 Jan 2022 09:29:24 +0100 Subject: [PATCH] Added option to explicitly not use fx variables in preprocessors (#1416) * Added option to explicitly not use fx variables in preprocessors * Updated doc * Updated doc * Deprecated always_use_ne_masks for mask_landsea * Always use fx_variables: null in doc * Moved Deprecation warning to dedicated exceptions module --- doc/recipe/preprocessor.rst | 17 +++- esmvalcore/_recipe.py | 23 +++-- esmvalcore/exceptions.py | 4 + esmvalcore/preprocessor/_mask.py | 19 ++++ .../preprocessor/_mask/test_mask.py | 11 ++- tests/integration/test_recipe.py | 93 +++++++++++++++++++ 6 files changed, 150 insertions(+), 17 deletions(-) diff --git a/doc/recipe/preprocessor.rst b/doc/recipe/preprocessor.rst index f2a7b251e2..d4644d5b89 100644 --- a/doc/recipe/preprocessor.rst +++ b/doc/recipe/preprocessor.rst @@ -195,11 +195,11 @@ Preprocessor Default fx variab :ref:`weighting_landsea_fraction` ``sftlf``, ``sftof`` ============================================================== ===================== -If no ``fx_variables`` are specified for these preprocessors, the fx variables -in the second column are used. If given, the ``fx_variables`` argument -specifies the fx variables that the user wishes to input to the corresponding -preprocessor function. The user may specify these by simply adding the names of -the variables, e.g., +If the option ``fx_variables`` is not explicitly specified for these +preprocessors, the default fx variables in the second column are automatically +used. If given, the ``fx_variables`` argument specifies the fx variables that +the user wishes to input to the corresponding preprocessor function. The user +may specify these by simply adding the names of the variables, e.g., .. code-block:: yaml @@ -247,6 +247,13 @@ available tables of the specified project. a given dataset) fx files are found in more than one table, ``mip`` needs to be specified, otherwise an error is raised. +.. note:: + To explicitly **not** use any fx variables in a preprocessor, use + ``fx_variables: null``. While some of the preprocessors mentioned above do + work without fx variables (e.g., ``area_statistics`` or ``mask_landsea`` + with datasets that have regular latitude/longitude grids), using this option + is **not** recommended. + Internally, the required ``fx_variables`` are automatically loaded by the preprocessor step ``add_fx_variables`` which also checks them against CMOR standards and adds them either as ``cell_measure`` (see `CF conventions on cell diff --git a/esmvalcore/_recipe.py b/esmvalcore/_recipe.py index 2ae1cca868..afe7cc3bbd 100644 --- a/esmvalcore/_recipe.py +++ b/esmvalcore/_recipe.py @@ -507,14 +507,12 @@ def _fx_list_to_dict(fx_vars): def _update_fx_settings(settings, variable, config_user): """Update fx settings depending on the needed method.""" - - # get fx variables either from user defined attribute or fixed - def _get_fx_vars_from_attribute(step_settings, step_name): - user_fx_vars = step_settings.get('fx_variables') - if isinstance(user_fx_vars, list): - user_fx_vars = _fx_list_to_dict(user_fx_vars) - step_settings['fx_variables'] = user_fx_vars - if not user_fx_vars: + # Add default values to the option 'fx_variables' if it is not explicitly + # specified and transform fx variables to dicts + def _update_fx_vars_in_settings(step_settings, step_name): + """Update fx_variables option in the settings.""" + # Add default values for fx_variables + if 'fx_variables' not in step_settings: default_fx = { 'area_statistics': { 'areacella': None, @@ -538,13 +536,20 @@ def _get_fx_vars_from_attribute(step_settings, step_name): default_fx['weighting_landsea_fraction']['sftof'] = None step_settings['fx_variables'] = default_fx[step_name] + # Transform fx variables to dicts + user_fx_vars = step_settings['fx_variables'] + if user_fx_vars is None: + step_settings['fx_variables'] = {} + elif isinstance(user_fx_vars, list): + step_settings['fx_variables'] = _fx_list_to_dict(user_fx_vars) + fx_steps = [ 'mask_landsea', 'mask_landseaice', 'weighting_landsea_fraction', 'area_statistics', 'volume_statistics' ] for step_name in settings: if step_name in fx_steps: - _get_fx_vars_from_attribute(settings[step_name], step_name) + _update_fx_vars_in_settings(settings[step_name], step_name) _update_fx_files(step_name, settings, variable, config_user, settings[step_name]['fx_variables']) # Remove unused attribute in 'fx_steps' preprocessors. diff --git a/esmvalcore/exceptions.py b/esmvalcore/exceptions.py index 1b756325c1..6013d29292 100644 --- a/esmvalcore/exceptions.py +++ b/esmvalcore/exceptions.py @@ -16,3 +16,7 @@ def __str__(self): class InputFilesNotFound(RecipeError): """Files that are required to run the recipe have not been found.""" + + +class ESMValCoreDeprecationWarning(UserWarning): + """Custom deprecation warning.""" diff --git a/esmvalcore/preprocessor/_mask.py b/esmvalcore/preprocessor/_mask.py index 32544a1b7f..05089da609 100644 --- a/esmvalcore/preprocessor/_mask.py +++ b/esmvalcore/preprocessor/_mask.py @@ -7,6 +7,7 @@ import logging import os +import warnings import cartopy.io.shapereader as shpreader import dask.array as da @@ -16,6 +17,8 @@ from iris.analysis import Aggregator from iris.util import rolling_window +from esmvalcore.exceptions import ESMValCoreDeprecationWarning + logger = logging.getLogger(__name__) @@ -81,6 +84,14 @@ def mask_landsea(cube, mask_out, always_use_ne_mask=False): always apply Natural Earths mask, regardless if fx files are available or not. + .. warning:: + This option has been deprecated in ESMValCore version 2.5. To + always use Natural Earth masks, either explicitly remove all + ``ancillary_variables`` from the input cube (when this function is + used directly) or specify ``fx_variables: null`` as option for this + preprocessor in the recipe (when this function is used as part of + ESMValTool). + Returns ------- iris.cube.Cube @@ -132,6 +143,14 @@ def mask_landsea(cube, mask_out, always_use_ne_mask=False): "yet implemented, land-sea mask not applied.") raise ValueError(msg) else: + deprecation_msg = ( + "The option ``always_use_ne_masks``' has been deprecated in " + "ESMValCore version 2.5. To always use Natural Earth masks, " + "either explicitly remove all ``ancillary_variables`` from the " + "input cube (when this function is used directly) or specify " + "``fx_variables: null`` as option for this preprocessor in the " + "recipe (when this function is used as part of ESMValTool).") + warnings.warn(deprecation_msg, ESMValCoreDeprecationWarning) if cube.coord('longitude').points.ndim < 2: cube = _mask_with_shp(cube, shapefiles[mask_out], [ 0, diff --git a/tests/integration/preprocessor/_mask/test_mask.py b/tests/integration/preprocessor/_mask/test_mask.py index d0bd8b7263..255e9d5547 100644 --- a/tests/integration/preprocessor/_mask/test_mask.py +++ b/tests/integration/preprocessor/_mask/test_mask.py @@ -7,13 +7,18 @@ """ import iris +import iris.fileformats import numpy as np import pytest from esmvalcore.cmor.check import CheckLevels -from esmvalcore.preprocessor import (PreprocessorFile, mask_fillvalues, - mask_landsea, mask_landseaice, - add_fx_variables) +from esmvalcore.preprocessor import ( + PreprocessorFile, + add_fx_variables, + mask_fillvalues, + mask_landsea, + mask_landseaice, +) from tests import assert_array_equal diff --git a/tests/integration/test_recipe.py b/tests/integration/test_recipe.py index f1cf8e99e1..5e9c65dd08 100644 --- a/tests/integration/test_recipe.py +++ b/tests/integration/test_recipe.py @@ -2116,6 +2116,99 @@ def test_landmask(tmp_path, patched_datafinder, config_user): assert len(fx_variables) == 2 +def test_empty_fxvar_none(tmp_path, patched_datafinder, config_user): + """Test that no fx variables are added if explicitly specified.""" + content = dedent(""" + preprocessors: + landmask: + mask_landsea: + mask_out: sea + fx_variables: null + diagnostics: + diagnostic_name: + variables: + gpp: + preprocessor: landmask + project: CMIP5 + mip: Lmon + exp: historical + start_year: 2000 + end_year: 2005 + ensemble: r1i1p1 + additional_datasets: + - {dataset: CanESM2} + scripts: null + """) + recipe = get_recipe(tmp_path, content, config_user) + + # Check that no custom fx variables are present + task = recipe.tasks.pop() + product = task.products.pop() + assert product.settings['add_fx_variables']['fx_variables'] == {} + + +def test_empty_fxvar_list(tmp_path, patched_datafinder, config_user): + """Test that no fx variables are added if explicitly specified.""" + content = dedent(""" + preprocessors: + landmask: + mask_landsea: + mask_out: sea + fx_variables: [] + diagnostics: + diagnostic_name: + variables: + gpp: + preprocessor: landmask + project: CMIP5 + mip: Lmon + exp: historical + start_year: 2000 + end_year: 2005 + ensemble: r1i1p1 + additional_datasets: + - {dataset: CanESM2} + scripts: null + """) + recipe = get_recipe(tmp_path, content, config_user) + + # Check that no custom fx variables are present + task = recipe.tasks.pop() + product = task.products.pop() + assert product.settings['add_fx_variables']['fx_variables'] == {} + + +def test_empty_fxvar_dict(tmp_path, patched_datafinder, config_user): + """Test that no fx variables are added if explicitly specified.""" + content = dedent(""" + preprocessors: + landmask: + mask_landsea: + mask_out: sea + fx_variables: {} + diagnostics: + diagnostic_name: + variables: + gpp: + preprocessor: landmask + project: CMIP5 + mip: Lmon + exp: historical + start_year: 2000 + end_year: 2005 + ensemble: r1i1p1 + additional_datasets: + - {dataset: CanESM2} + scripts: null + """) + recipe = get_recipe(tmp_path, content, config_user) + + # Check that no custom fx variables are present + task = recipe.tasks.pop() + product = task.products.pop() + assert product.settings['add_fx_variables']['fx_variables'] == {} + + def test_user_defined_fxvar(tmp_path, patched_datafinder, config_user): content = dedent(""" preprocessors: