Repository: ESMValGroup/ESMValCore

Issue:
Dataset problem: MCM-UA-1-0 has missing 10m height for variable `uas`
**Describe the dataset issue**

- Data file has been changed by you in any way (if you answer yes the issue will be void and closed, we are not
supporting custom-made datasets that present problems, it is your resposability to fix those problems):
- Project (CMIP5/CMIP6/OBS/obs4mips/etc): CMIP6
- Full dataset description (fill out as many as you know, please):
  - dataset: MCM-UA-1-0
  - experiment: historical
  - mip: Amon
  - grid: gn
  - variable used: uas
  - data version:
- Problems encountered (please describe what the actual problems are: e.g. wrong standard name, issue with dimensions): `esmvalcore.cmor.check.CMORCheckError: There were errors in variable uas:
height10m: does not exist`
- Pointer to existing copy of data on ESGF node (it would be very useful if you could provide a physical
fullpath to the file(s) that are causing the problem, e.g. on CEDA Jasmin or DKRZ):
/mnt/lustre02/work/ik1017/CMIP6/data/CMIP6/CMIP/UA/MCM-UA-1-0/historical/r1i1p1f1/Amon/uas/gn/v20190731/uas_Amon_MCM-UA-1-0_historical_r1i1p1f1_gn_185001-201412.nc

**Assign the right people**
If you are already a member of the ESMValTool GitHub project, please assign Valeriu Predoi (valeriupredoi) and
Klaus Zimmermann (zklaus) to the issue. They will then check the issue raised and propagate it further to the
data model developers.

Cheers :beer:


Discussion:
remi-kazeroni: > @remi-kazeroni and @LisaBock please ping me when you change your various fixes PRs from Draft to RfR - these things are easy to review and I'll merge them very quickly 👍

@valeriupredoi thanks a lot for offering your help for the review of the fixes 👍  We'll mark them RfR asap. This one is ready. Just to let you know, I had a seg fault on circleci with the latest commit but it disappeared simply by rerunning the tests 🍺 

Patch:
diff --git a/esmvalcore/_recipe.py b/esmvalcore/_recipe.py
index 6db4df455..a57dac967 100644
--- a/esmvalcore/_recipe.py
+++ b/esmvalcore/_recipe.py
@@ -532,9 +532,7 @@ def _get_ancestors(variable, config_user):
     logger.info("Using input files for variable %s of dataset %s:\n%s",
                 variable['short_name'], variable['dataset'],
                 '\n'.join(input_files))
-    if (not config_user.get('skip-nonexistent')
-            or variable['dataset'] == variable.get('reference_dataset')):
-        check.data_availability(input_files, variable, dirnames, filenames)
+    check.data_availability(input_files, variable, dirnames, filenames)
 
     # Set up provenance tracking
     for i, filename in enumerate(input_files):
@@ -673,6 +671,16 @@ def _match_products(products, variables):
     return grouped_products
 
 
+def _allow_skipping(ancestors, variable, config_user):
+    """Allow skipping of datasets."""
+    allow_skipping = all([
+        config_user.get('skip-nonexistent'),
+        not ancestors,
+        variable['dataset'] != variable.get('reference_dataset'),
+    ])
+    return allow_skipping
+
+
 def _get_preprocessor_products(variables, profile, order, ancestor_products,
                                config_user, name):
     """Get preprocessor product definitions for a set of datasets.
@@ -714,7 +722,7 @@ def _get_preprocessor_products(variables, profile, order, ancestor_products,
             try:
                 ancestors = _get_ancestors(variable, config_user)
             except RecipeError as ex:
-                if config_user.get('skip-nonexistent') and not ancestors:
+                if _allow_skipping(ancestors, variable, config_user):
                     logger.info("Skipping: %s", ex.message)
                 else:
                     missing_vars.add(ex.message)
diff --git a/esmvalcore/cmor/_fixes/cmip6/mcm_ua_1_0.py b/esmvalcore/cmor/_fixes/cmip6/mcm_ua_1_0.py
index 08189338a..c31a5e7c5 100644
--- a/esmvalcore/cmor/_fixes/cmip6/mcm_ua_1_0.py
+++ b/esmvalcore/cmor/_fixes/cmip6/mcm_ua_1_0.py
@@ -85,6 +85,30 @@ class AllVars(Fix):
         return cubes
 
 
+class Omon(Fix):
+    """Fixes for ocean variables."""
+
+    def fix_metadata(self, cubes):
+        """Fix ocean depth coordinate.
+
+        Parameters
+        ----------
+        cubes: iris CubeList
+            List of cubes to fix
+
+        Returns
+        -------
+        iris.cube.CubeList
+
+        """
+        for cube in cubes:
+            if cube.coords(axis='Z'):
+                z_coord = cube.coord(axis='Z')
+                if z_coord.standard_name is None:
+                    fix_ocean_depth_coord(cube)
+        return cubes
+
+
 class Tas(Fix):
     """Fixes for tas."""
 
@@ -98,33 +122,30 @@ class Tas(Fix):
 
         Returns
         -------
-        iris.cube.Cube
+        iris.cube.CubeList
 
         """
         cube = self.get_cube_from_list(cubes)
         add_scalar_height_coord(cube, 2.0)
-        return [cube]
+        return cubes
 
 
-class Omon(Fix):
-    """Fixes for ocean variables."""
+class Uas(Fix):
+    """Fixes for uas."""
 
     def fix_metadata(self, cubes):
-        """Fix ocean depth coordinate.
+        """Add height (10m) coordinate.
 
         Parameters
         ----------
-        cubes: iris CubeList
-            List of cubes to fix
+        cubes : iris.cube.CubeList
+            Cubes to fix.
 
         Returns
         -------
         iris.cube.CubeList
 
         """
-        for cube in cubes:
-            if cube.coords(axis='Z'):
-                z_coord = cube.coord(axis='Z')
-                if z_coord.standard_name is None:
-                    fix_ocean_depth_coord(cube)
+        cube = self.get_cube_from_list(cubes)
+        add_scalar_height_coord(cube, 10.0)
         return cubes
diff --git a/esmvalcore/cmor/tables/custom/CMOR_tasaga.dat b/esmvalcore/cmor/tables/custom/CMOR_tasaga.dat
new file mode 100644
index 000000000..f62d34c5e
--- /dev/null
+++ b/esmvalcore/cmor/tables/custom/CMOR_tasaga.dat
@@ -0,0 +1,25 @@
+SOURCE: CMIP5 (adapted from tas)
+!============
+variable_entry:    tasaga
+!============
+modeling_realm:    atmos
+!----------------------------------
+! Variable attributes:
+!----------------------------------
+standard_name:     
+units:             K
+cell_methods:      time: mean
+cell_measures:     area: areacella
+long_name:         Global-mean Near-Surface Air Temperature Anomaly
+!----------------------------------
+! Additional variable information:
+!----------------------------------
+dimensions:        time
+out_name:          tasaga
+type:              real
+valid_min:         -20.0
+valid_max:         20.0
+ok_min_mean_abs:   -20
+ok_max_mean_abs:   20.
+!----------------------------------
+!
diff --git a/esmvalcore/preprocessor/_io.py b/esmvalcore/preprocessor/_io.py
index f508f0fa4..938e4b6f2 100644
--- a/esmvalcore/preprocessor/_io.py
+++ b/esmvalcore/preprocessor/_io.py
@@ -173,6 +173,8 @@ def _get_concatenation_error(cubes):
 
 def concatenate(cubes):
     """Concatenate all cubes after fixing metadata."""
+    if not cubes:
+        return cubes
     if len(cubes) == 1:
         return cubes[0]
 
@@ -227,7 +229,15 @@ def save(cubes, filename, optimize_access='', compress=False, alias='',
     str
         filename
 
+    Raises
+    ------
+    ValueError
+        cubes is empty.
+
     """
+    if not cubes:
+        raise ValueError(f"Cannot save empty cubes '{cubes}'")
+
     # Rename some arguments
     kwargs['target'] = filename
     kwargs['zlib'] = compress


Test patch:
diff --git a/tests/integration/cmor/_fixes/cmip6/test_mcm_ua_1_0.py b/tests/integration/cmor/_fixes/cmip6/test_mcm_ua_1_0.py
index 04202f524..a88694681 100644
--- a/tests/integration/cmor/_fixes/cmip6/test_mcm_ua_1_0.py
+++ b/tests/integration/cmor/_fixes/cmip6/test_mcm_ua_1_0.py
@@ -4,7 +4,7 @@ import numpy as np
 import pytest
 from cf_units import Unit
 
-from esmvalcore.cmor._fixes.cmip6.mcm_ua_1_0 import AllVars, Omon, Tas
+from esmvalcore.cmor._fixes.cmip6.mcm_ua_1_0 import AllVars, Omon, Tas, Uas
 from esmvalcore.cmor.fix import Fix
 from esmvalcore.cmor.table import get_var_info
 
@@ -48,6 +48,45 @@ def cubes():
     return iris.cube.CubeList([correct_cube, wrong_cube, scalar_cube])
 
 
+@pytest.fixture
+def uas_cubes():
+    correct_lat_coord = iris.coords.DimCoord([0.0],
+                                             var_name='lat',
+                                             standard_name=' latitude  ',
+                                             long_name='  latitude')
+    wrong_lat_coord = iris.coords.DimCoord([0.0],
+                                           var_name='latitudeCoord',
+                                           standard_name='  latitude',
+                                           long_name='latitude')
+    correct_lon_coord = iris.coords.DimCoord([0.0],
+                                             var_name='lon',
+                                             standard_name='  longitude  ',
+                                             long_name='longitude  ')
+    wrong_lon_coord = iris.coords.DimCoord([0.0],
+                                           var_name='longitudeCoord',
+                                           standard_name='longitude',
+                                           long_name='  longitude')
+    correct_cube = iris.cube.Cube(
+        [[10.0]],
+        var_name='uas',
+        standard_name='eastward_wind   ',
+        long_name='   East Near-Surface Wind   ',
+        dim_coords_and_dims=[(correct_lat_coord, 0), (correct_lon_coord, 1)],
+    )
+    wrong_cube = iris.cube.Cube(
+        [[10.0]],
+        var_name='ta',
+        standard_name='   air_temperature   ',
+        long_name='Air Temperature',
+        dim_coords_and_dims=[(wrong_lat_coord, 0), (wrong_lon_coord, 1)],
+        attributes={'parent_time_units': 'days since 0000-00-00 (noleap)'},
+    )
+    scalar_cube = iris.cube.Cube(0.0, var_name='ps',
+                                 standard_name='air_pressure   ',
+                                 long_name=' Air pressure  ')
+    return iris.cube.CubeList([correct_cube, wrong_cube, scalar_cube])
+
+
 @pytest.fixture
 def cubes_bounds():
     lat_coord = iris.coords.DimCoord([0.0],
@@ -87,6 +126,11 @@ def test_get_tas_fix():
     assert fix == [Tas(None), AllVars(None)]
 
 
+def test_get_uas_fix():
+    fix = Fix.get_fixes('CMIP6', 'MCM-UA-1-0', 'Amon', 'uas')
+    assert fix == [Uas(None), AllVars(None)]
+
+
 def test_allvars_fix_metadata(cubes):
     fix = AllVars(None)
     out_cubes = fix.fix_metadata(cubes)
@@ -160,6 +204,32 @@ def test_tas_fix_metadata(cubes):
     assert coord == height_coord
 
 
+def test_uas_fix_metadata(uas_cubes):
+    for cube in uas_cubes:
+        with pytest.raises(iris.exceptions.CoordinateNotFoundError):
+            cube.coord('height')
+    height_coord = iris.coords.AuxCoord(10.0,
+                                        var_name='height',
+                                        standard_name='height',
+                                        long_name='height',
+                                        units=Unit('m'),
+                                        attributes={'positive': 'up'})
+    vardef = get_var_info('CMIP6', 'Amon', 'uas')
+    fix = Uas(vardef)
+
+    # Check fix
+    out_cubes = fix.fix_metadata(uas_cubes)
+    assert out_cubes[0].var_name == 'uas'
+    coord = out_cubes[0].coord('height')
+    assert coord == height_coord
+
+    # Check that height coordinate is not added twice
+    out_cubes_2 = fix.fix_metadata(out_cubes)
+    assert out_cubes_2[0].var_name == 'uas'
+    coord = out_cubes_2[0].coord('height')
+    assert coord == height_coord
+
+
 @pytest.fixture
 def thetao_cubes():
     time_coord = iris.coords.DimCoord(
diff --git a/tests/integration/cmor/test_table.py b/tests/integration/cmor/test_table.py
index 0580fad4d..568dd6aab 100644
--- a/tests/integration/cmor/test_table.py
+++ b/tests/integration/cmor/test_table.py
@@ -381,3 +381,12 @@ class TestCustomInfo(unittest.TestCase):
         self.assertEqual(var.long_name,
                          'Near-Surface Air Temperature Uncertainty Range')
         self.assertEqual(var.units, 'K')
+
+    def test_get_variable_tasaga(self):
+        """Get tas variable."""
+        CustomInfo()
+        var = self.variables_info.get_variable('Amon', 'tasaga')
+        self.assertEqual(var.short_name, 'tasaga')
+        self.assertEqual(var.long_name,
+                         'Global-mean Near-Surface Air Temperature Anomaly')
+        self.assertEqual(var.units, 'K')
diff --git a/tests/integration/preprocessor/_io/test_concatenate.py b/tests/integration/preprocessor/_io/test_concatenate.py
index f8ef998f9..ce0d9529b 100644
--- a/tests/integration/preprocessor/_io/test_concatenate.py
+++ b/tests/integration/preprocessor/_io/test_concatenate.py
@@ -1,7 +1,7 @@
 """Integration tests for :func:`esmvalcore.preprocessor._io.concatenate`."""
 
-import warnings
 import unittest
+import warnings
 from unittest.mock import call
 
 import numpy as np
@@ -243,6 +243,12 @@ class TestConcatenate(unittest.TestCase):
         np.testing.assert_array_equal(
             concatenated.coord('time').points, np.array([1, 2, 3, 4, 5, 6]))
 
+    def test_concatenate_empty_cubes(self):
+        """Test concatenation with empty :class:`iris.cube.CubeList`."""
+        empty_cubes = CubeList([])
+        result = _io.concatenate(empty_cubes)
+        assert result is empty_cubes
+
     def test_concatenate_noop(self):
         """Test concatenation of a single cube."""
         concatenated = _io.concatenate([self.raw_cubes[0]])
diff --git a/tests/integration/preprocessor/_io/test_save.py b/tests/integration/preprocessor/_io/test_save.py
index cc6c98364..9ccb25efc 100644
--- a/tests/integration/preprocessor/_io/test_save.py
+++ b/tests/integration/preprocessor/_io/test_save.py
@@ -8,7 +8,7 @@ import iris
 import netCDF4
 import numpy as np
 from iris.coords import DimCoord
-from iris.cube import Cube
+from iris.cube import Cube, CubeList
 
 from esmvalcore.preprocessor import save
 
@@ -78,6 +78,13 @@ class TestSave(unittest.TestCase):
         self.assertEqual(sample_filters['complevel'], 4)
         handler.close()
 
+    def test_fail_empty_cubes(self):
+        """Test save fails if empty cubes is provided."""
+        (_, filename) = self._create_sample_cube()
+        empty_cubes = CubeList([])
+        with self.assertRaises(ValueError):
+            save(empty_cubes, filename)
+
     def test_fail_without_filename(self):
         """Test save fails if filename is not provided."""
         cube, _ = self._create_sample_cube()
diff --git a/tests/unit/test_recipe.py b/tests/unit/test_recipe.py
index 1ed187592..885684cee 100644
--- a/tests/unit/test_recipe.py
+++ b/tests/unit/test_recipe.py
@@ -1,6 +1,6 @@
 import pytest
 
-from esmvalcore._recipe import Recipe
+from esmvalcore._recipe import Recipe, _allow_skipping
 from esmvalcore._recipe_checks import RecipeError
 
 
@@ -40,3 +40,37 @@ class TestRecipe:
 
         with pytest.raises(RecipeError):
             Recipe._expand_ensemble(datasets)
+
+
+VAR_A = {'dataset': 'A'}
+VAR_A_REF_A = {'dataset': 'A', 'reference_dataset': 'A'}
+VAR_A_REF_B = {'dataset': 'A', 'reference_dataset': 'B'}
+
+
+TEST_ALLOW_SKIPPING = [
+    ([], VAR_A, {}, False),
+    ([], VAR_A, {'skip-nonexistent': False}, False),
+    ([], VAR_A, {'skip-nonexistent': True}, True),
+    ([], VAR_A_REF_A, {}, False),
+    ([], VAR_A_REF_A, {'skip-nonexistent': False}, False),
+    ([], VAR_A_REF_A, {'skip-nonexistent': True}, False),
+    ([], VAR_A_REF_B, {}, False),
+    ([], VAR_A_REF_B, {'skip-nonexistent': False}, False),
+    ([], VAR_A_REF_B, {'skip-nonexistent': True}, True),
+    (['A'], VAR_A, {}, False),
+    (['A'], VAR_A, {'skip-nonexistent': False}, False),
+    (['A'], VAR_A, {'skip-nonexistent': True}, False),
+    (['A'], VAR_A_REF_A, {}, False),
+    (['A'], VAR_A_REF_A, {'skip-nonexistent': False}, False),
+    (['A'], VAR_A_REF_A, {'skip-nonexistent': True}, False),
+    (['A'], VAR_A_REF_B, {}, False),
+    (['A'], VAR_A_REF_B, {'skip-nonexistent': False}, False),
+    (['A'], VAR_A_REF_B, {'skip-nonexistent': True}, False),
+]
+
+
+@pytest.mark.parametrize('ancestors,var,cfg,out', TEST_ALLOW_SKIPPING)
+def test_allow_skipping(ancestors, var, cfg, out):
+    """Test ``_allow_skipping``."""
+    result = _allow_skipping(ancestors, var, cfg)
+    assert result is out