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

Author: schlunma

doc/recipe/preprocessor.rst

@@ -195,11 +195,11 @@ Preprocessor                                                   Default fx variab
 :ref:`weighting_landsea_fraction<land/sea fraction weighting>` ``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

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.

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."""

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,

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
 
 

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: