Repository: ESMValGroup/ESMValCore

Issue:
Running with CMIP6 data and OBS data fails if using mip: AERmonZ
...since OBS data is using `cmor.table.CMIP5Info` that doesn't know of `AERmonZ` since that is not a CMIP5 mip

Discussion:

Patch:
diff --git a/esmvalcore/_recipe.py b/esmvalcore/_recipe.py
index 4b240e91b..36aba2043 100644
--- a/esmvalcore/_recipe.py
+++ b/esmvalcore/_recipe.py
@@ -118,16 +118,15 @@ def _add_cmor_info(variable, override=False):
     if derive and table_entry is None:
         custom_table = CMOR_TABLES['custom']
         table_entry = custom_table.get_variable(mip, short_name)
+        table_entry.copy()
+        mip_info = CMOR_TABLES[cmor_table].get_table(mip)
+        table_entry.frequency = mip_info.frequency
 
     if table_entry is None:
         raise RecipeError(
             "Unable to load CMOR table '{}' for variable '{}' with mip '{}'".
             format(cmor_table, short_name, mip))
 
-    mip_info = CMOR_TABLES[cmor_table].get_table(mip)
-    if mip_info:
-        table_entry.frequency = mip_info.frequency
-
     for key in cmor_keys:
         if key not in variable or override:
             value = getattr(table_entry, key, None)
diff --git a/esmvalcore/cmor/table.py b/esmvalcore/cmor/table.py
index 805ede3a6..cb126d89a 100644
--- a/esmvalcore/cmor/table.py
+++ b/esmvalcore/cmor/table.py
@@ -4,6 +4,8 @@ CMOR information reader for ESMValTool.
 Read variable information from CMOR 2 and CMOR 3 tables and make it easily
 available for the other components of ESMValTool
 """
+from functools import total_ordering
+import copy
 import errno
 import glob
 import json
@@ -42,11 +44,11 @@ def read_cmor_tables(cfg_developer):
             default = custom
         if cmor_type == 'CMIP5':
             CMOR_TABLES[table] = CMIP5Info(
-                table_path, default=default,
+                table_path, default=default, strict=cmor_strict,
             )
         elif cmor_type == 'CMIP6':
             CMOR_TABLES[table] = CMIP6Info(
-                table_path, default=default,
+                table_path, default=default, strict=cmor_strict,
             )
 
 
@@ -61,6 +63,13 @@ class CMIP6Info(object):
     cmor_tables_path: basestring
         Path to the folder containing the Tables folder with the json files
 
+    default: object
+        Default table to look variables on if not found
+
+    strict: bool
+        If False, will look for a variable in other tables if it can not be
+        found in the requested one
+
     """
 
     _CMIP_5to6_varname = {
@@ -69,13 +78,14 @@ class CMIP6Info(object):
         'tro3': 'o3',
     }
 
-    def __init__(self, cmor_tables_path, default=None):
+    def __init__(self, cmor_tables_path, default=None, strict=True):
         cmor_tables_path = self._get_cmor_path(cmor_tables_path)
 
         self._cmor_folder = os.path.join(cmor_tables_path, 'Tables')
         self.default = default
 
         self.tables = {}
+        self.strict = strict
 
         self._load_coordinates()
         for json_file in glob.glob(os.path.join(self._cmor_folder, '*.json')):
@@ -183,9 +193,24 @@ class CMIP6Info(object):
             if short_name in CMIP6Info._CMIP_5to6_varname:
                 new_short_name = CMIP6Info._CMIP_5to6_varname[short_name]
                 return self.get_variable(table, new_short_name)
-            if self.default:
-                return self.default.get_variable(table, short_name)
-            return None
+
+            var_info = None
+            if not self.strict:
+                for table_vars in sorted(self.tables.values()):
+                    if short_name in table_vars:
+                        var_info = table_vars[short_name]
+                        break
+
+            if not var_info and self.default:
+                var_info = self.default.get_variable(table, short_name)
+
+            if var_info:
+                mip_info = self.get_table(table)
+                if mip_info:
+                    var_info = var_info.copy()
+                    var_info.frequency = mip_info.frequency
+
+            return var_info
 
     @staticmethod
     def _is_table(table_data):
@@ -196,6 +221,7 @@ class CMIP6Info(object):
         return True
 
 
+@total_ordering
 class TableInfo(dict):
     """Container class for storing a CMOR table."""
 
@@ -206,6 +232,18 @@ class TableInfo(dict):
         self.frequency = ''
         self.realm = ''
 
+    def __eq__(self, other):
+        return (self.name, self.frequency, self.realm) == \
+            (other.name, other.frequency, other.realm)
+
+    def __ne__(self, other):
+        return (self.name, self.frequency, self.realm) != \
+            (other.name, other.frequency, other.realm)
+
+    def __lt__(self, other):
+        return (self.name, self.frequency, self.realm) < \
+            (other.name, other.frequency, other.realm)
+
 
 class JsonInfo(object):
     """
@@ -297,6 +335,17 @@ class VariableInfo(JsonInfo):
 
         self._json_data = None
 
+    def copy(self):
+        """
+        Return a shalow copy of VariableInfo.
+
+        Returns
+        -------
+        VariableInfo
+           Shallow copy of this object
+        """
+        return copy.copy(self)
+
     def read_json(self, json_data):
         """
         Read variable information from json.
@@ -405,9 +454,16 @@ class CMIP5Info(object):
     cmor_tables_path: basestring
        Path to the folder containing the Tables folder with the json files
 
+    default: object
+        Default table to look variables on if not found
+
+    strict: bool
+        If False, will look for a variable in other tables if it can not be
+        found in the requested one
+
     """
 
-    def __init__(self, cmor_tables_path, default=None):
+    def __init__(self, cmor_tables_path, default=None, strict=True):
         cmor_tables_path = self._get_cmor_path(cmor_tables_path)
 
         self._cmor_folder = os.path.join(cmor_tables_path, 'Tables')
@@ -418,6 +474,7 @@ class CMIP5Info(object):
         self.tables = {}
         self.coords = {}
         self.default = default
+        self.strict = strict
         self._current_table = None
         self._last_line_read = None
 
@@ -555,8 +612,18 @@ class CMIP5Info(object):
 
         """
         var_info = self.tables.get(table, {}).get(short_name, None)
+        if not var_info and not self.strict:
+            for table_vars in sorted(self.tables.values()):
+                if short_name in table_vars:
+                    var_info = table_vars[short_name]
+                    break
         if not var_info and self.default:
-            return self.default.get_variable(table, short_name)
+            var_info = self.default.get_variable(table, short_name)
+        if var_info:
+            mip_info = self.get_table(table)
+            var_info.copy()
+            if mip_info:
+                var_info.frequency = mip_info.frequency
         return var_info
 
 


Test patch:
diff --git a/tests/integration/cmor/test_table.py b/tests/integration/cmor/test_table.py
index 84540a297..393413c08 100644
--- a/tests/integration/cmor/test_table.py
+++ b/tests/integration/cmor/test_table.py
@@ -18,6 +18,9 @@ class TestCMIP6Info(unittest.TestCase):
         """
         cls.variables_info = CMIP6Info('cmip6', default=CustomInfo())
 
+    def setUp(self):
+        self.variables_info.strict = True
+
     def test_custom_tables_location(self):
         """Test constructor with custom tables location."""
         cwd = os.path.dirname(os.path.realpath(__file__))
@@ -38,7 +41,25 @@ class TestCMIP6Info(unittest.TestCase):
 
     def test_get_bad_variable(self):
         """Get none if a variable is not in the given table."""
-        self.assertIsNone(self.variables_info.get_variable('Omon', 'tas'))
+        self.assertIsNone(self.variables_info.get_variable('Omon', 'ta'))
+
+    def test_omon_ta_fail_if_strict(self):
+        """Get ta fails with Omon if strict."""
+        self.assertIsNone(self.variables_info.get_variable('Omon', 'ta'))
+
+    def test_omon_ta_succes_if_strict(self):
+        """Get ta does not fail with AERMonZ if not strict."""
+        self.variables_info.strict = False
+        var = self.variables_info.get_variable('Omon', 'ta')
+        self.assertEqual(var.short_name, 'ta')
+        self.assertEqual(var.frequency, 'mon')
+
+    def test_omon_toz_succes_if_strict(self):
+        """Get troz does not fail with Omon if not strict."""
+        self.variables_info.strict = False
+        var = self.variables_info.get_variable('Omon', 'toz')
+        self.assertEqual(var.short_name, 'toz')
+        self.assertEqual(var.frequency, 'mon')
 
 
 class Testobs4mipsInfo(unittest.TestCase):
@@ -86,6 +107,9 @@ class TestCMIP5Info(unittest.TestCase):
         """
         cls.variables_info = CMIP5Info('cmip5', default=CustomInfo())
 
+    def setUp(self):
+        self.variables_info.strict = True
+
     def test_custom_tables_location(self):
         """Test constructor with custom tables location."""
         cwd = os.path.dirname(os.path.realpath(__file__))
@@ -103,6 +127,24 @@ class TestCMIP5Info(unittest.TestCase):
         """Get none if a variable is not in the given table."""
         self.assertIsNone(self.variables_info.get_variable('Omon', 'tas'))
 
+    def test_aermon_ta_fail_if_strict(self):
+        """Get ta fails with AERMonZ if strict."""
+        self.assertIsNone(self.variables_info.get_variable('Omon', 'ta'))
+
+    def test_aermon_ta_succes_if_strict(self):
+        """Get ta does not fail with Omon if not strict."""
+        self.variables_info.strict = False
+        var = self.variables_info.get_variable('Omon', 'ta')
+        self.assertEqual(var.short_name, 'ta')
+        self.assertEqual(var.frequency, 'mon')
+
+    def test_omon_toz_succes_if_strict(self):
+        """Get troz does not fail with Omon if not strict."""
+        self.variables_info.strict = False
+        var = self.variables_info.get_variable('Omon', 'toz')
+        self.assertEqual(var.short_name, 'toz')
+        self.assertEqual(var.frequency, 'mon')
+
 
 class TestCustomInfo(unittest.TestCase):
     """Test for the custom info class."""