Repository: ESMValGroup/ESMValCore

Issue:
obs4mips CMOR table not used
At the moment, the obs4mips CMOR table is present but not used, the CMIP6 table is used instead to look up obs4mips variables.

Discussion:

Patch:
diff --git a/esmvalcore/cmor/table.py b/esmvalcore/cmor/table.py
index 09e827016..f179f8193 100644
--- a/esmvalcore/cmor/table.py
+++ b/esmvalcore/cmor/table.py
@@ -34,9 +34,10 @@ def read_cmor_tables(cfg_developer):
         project = cfg_developer[table]
 
         cmor_type = project.get('cmor_type', 'CMIP5')
-        table_path = project.get('cmor_tables', cmor_type.lower())
+        table_path = project.get('cmor_path', cmor_type.lower())
         table_path = os.path.expandvars(os.path.expanduser(table_path))
         cmor_strict = project.get('cmor_strict', True)
+        default_table_prefix = project.get('cmor_default_table_prefix', '')
 
         if cmor_type == 'CMIP5':
             CMOR_TABLES[table] = CMIP5Info(
@@ -45,6 +46,7 @@ def read_cmor_tables(cfg_developer):
         elif cmor_type == 'CMIP6':
             CMOR_TABLES[table] = CMIP6Info(
                 table_path, default=custom, strict=cmor_strict,
+                default_table_prefix=default_table_prefix
             )
 
 
@@ -76,7 +78,8 @@ class CMIP6Info(object):
         'vsi': 'siv',
     }
 
-    def __init__(self, cmor_tables_path, default=None, strict=True):
+    def __init__(self, cmor_tables_path, default=None, strict=True,
+                 default_table_prefix=''):
         cmor_tables_path = self._get_cmor_path(cmor_tables_path)
 
         self._cmor_folder = os.path.join(cmor_tables_path, 'Tables')
@@ -84,6 +87,7 @@ class CMIP6Info(object):
             self._load_controlled_vocabulary()
         self.default = default
         self.strict = strict
+        self.default_table_prefix = default_table_prefix
 
         self.tables = {}
         self.var_to_freq = {}
@@ -216,15 +220,18 @@ class CMIP6Info(object):
             found, returns None if not
 
         """
-        return self.tables.get(table)
+        try:
+            return self.tables[table]
+        except KeyError:
+            return self.tables.get(''.join((self.default_table_prefix, table)))
 
-    def get_variable(self, table, short_name, derived=False):
+    def get_variable(self, table_name, short_name, derived=False):
         """
         Search and return the variable info.
 
         Parameters
         ----------
-        table: basestring
+        table_name: basestring
             Table name
         short_name: basestring
             Variable's short name
@@ -238,29 +245,33 @@ class CMIP6Info(object):
             found, returns None if not
 
         """
-        try:
-            return self.tables[table][short_name]
-        except KeyError:
-            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, derived)
-
-            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 (not self.strict or derived):
-                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
+        table = self.get_table(table_name)
+        if table:
+            try:
+                return table[short_name]
+            except KeyError:
+                pass
 
-            return var_info
+        if short_name in CMIP6Info._CMIP_5to6_varname:
+            new_short_name = CMIP6Info._CMIP_5to6_varname[short_name]
+            return self.get_variable(table_name, new_short_name, derived)
+
+        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 (not self.strict or derived):
+            var_info = self.default.get_variable(table_name, short_name)
+
+        if var_info:
+            mip_info = self.get_table(table_name)
+            if mip_info:
+                var_info = var_info.copy()
+                var_info.frequency = mip_info.frequency
+
+        return var_info
 
     @staticmethod
     def _is_table(table_data):
diff --git a/esmvalcore/config-developer.yml b/esmvalcore/config-developer.yml
index 662495a41..10a265778 100644
--- a/esmvalcore/config-developer.yml
+++ b/esmvalcore/config-developer.yml
@@ -19,6 +19,7 @@
 ---
 
 CMIP6:
+  cmor_strict: true
   input_dir:
     default: '/'
     BADC: '[activity]/[institute]/[dataset]/[exp]/[ensemble]/[mip]/[short_name]/[grid]/[latestversion]'
@@ -156,7 +157,7 @@ OBS6:
   cmor_type: 'CMIP6'
 
 obs4mips:
-  cmor_strict: false
+  cmor_strict: true
   input_dir:
     default: 'Tier[tier]/[dataset]'
     RCAST: '/'
@@ -168,6 +169,7 @@ obs4mips:
   output_file: '[project]_[dataset]_[level]_[version]_[short_name]_[start_year]-[end_year]'
   cmor_type: 'CMIP6'
   cmor_path: 'obs4mips'
+  cmor_default_table_prefix: 'obs4MIPs_'
 
 ana4mips:
   cmor_strict: false
@@ -177,6 +179,7 @@ ana4mips:
   input_file: '[short_name]_[mip]_[type]_[dataset]_*.nc'
   output_file: '[project]_[mip]_[type]_[dataset]_[short_name]_[start_year]-[end_year]'
   cmor_type: 'CMIP5'
+  # TODO: add cmor_path and table and set cmor_strict to true
 
 EMAC:
   input_dir:


Test patch:
diff --git a/tests/integration/cmor/test_read_cmor_tables.py b/tests/integration/cmor/test_read_cmor_tables.py
new file mode 100644
index 000000000..22f1a3a42
--- /dev/null
+++ b/tests/integration/cmor/test_read_cmor_tables.py
@@ -0,0 +1,30 @@
+from pathlib import Path
+
+from esmvalcore._config import read_config_developer_file
+from esmvalcore.cmor.table import CMOR_TABLES
+from esmvalcore.cmor.table import __file__ as root
+from esmvalcore.cmor.table import read_cmor_tables
+
+
+def test_read_cmor_tables():
+    """Test that the funcion `read_cmor_tables` loads the tables correctly."""
+    # Read the tables
+    read_cmor_tables(read_config_developer_file())
+
+    table_path = Path(root).parent / 'tables'
+
+    for project in 'CMIP5', 'CMIP6', 'obs4mips':
+        table = CMOR_TABLES[project]
+        assert Path(
+            table._cmor_folder) == table_path / project.lower() / 'Tables'
+        assert table.strict is True
+
+    project = 'OBS'
+    table = CMOR_TABLES[project]
+    assert Path(table._cmor_folder) == table_path / 'cmip5' / 'Tables'
+    assert table.strict is False
+
+    project = 'OBS6'
+    table = CMOR_TABLES[project]
+    assert Path(table._cmor_folder) == table_path / 'cmip6' / 'Tables'
+    assert table.strict is False
diff --git a/tests/integration/cmor/test_table.py b/tests/integration/cmor/test_table.py
index 7b82636b7..be0999a01 100644
--- a/tests/integration/cmor/test_table.py
+++ b/tests/integration/cmor/test_table.py
@@ -107,6 +107,7 @@ class Testobs4mipsInfo(unittest.TestCase):
             cmor_tables_path='obs4mips',
             default=CustomInfo(),
             strict=True,
+            default_table_prefix='obs4MIPs_'
         )
 
     def setUp(self):
@@ -134,6 +135,18 @@ class Testobs4mipsInfo(unittest.TestCase):
         self.assertEqual(var.short_name, 'ndviStderr')
         self.assertEqual(var.frequency, 'mon')
 
+    def test_get_variable_hus(self):
+        """Get hus variable."""
+        var = self.variables_info.get_variable('obs4MIPs_Amon', 'hus')
+        self.assertEqual(var.short_name, 'hus')
+        self.assertEqual(var.frequency, 'mon')
+
+    def test_get_variable_hus_default_prefix(self):
+        """Get hus variable."""
+        var = self.variables_info.get_variable('Amon', 'hus')
+        self.assertEqual(var.short_name, 'hus')
+        self.assertEqual(var.frequency, 'mon')
+
     def test_get_variable_from_custom(self):
         """Get a variable from default."""
         var = self.variables_info.get_variable(