From 96ccc04711c391ffd7a693f0788851a63a07aa00 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Apr 2026 22:26:32 +0000 Subject: [PATCH 1/3] feat: support nested variable entries in CMOR tables Agent-Logs-Url: https://github.com/ilaflott/cmor/sessions/789e2979-03f5-480c-a8d7-6487d07b9c72 Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com> --- Src/cmor_tables.c | 66 ++++++++++++++++++++- Test/test_cmor_CMIP7.py | 76 ++++++++++++++++++++++++ Test/test_python_branded_variable.py | 88 +++++++++++++++++++++++++++- 3 files changed, 226 insertions(+), 4 deletions(-) diff --git a/Src/cmor_tables.c b/Src/cmor_tables.c index 5cf32a86..e771854c 100644 --- a/Src/cmor_tables.c +++ b/Src/cmor_tables.c @@ -285,6 +285,49 @@ int cmor_set_variable_entry(cmor_table_t * table, return (0); } +static int cmor_variable_entry_uses_nested_brands(json_object *json) +{ + json_object_object_foreach(json, attr, value) { + if (attr[0] == '#') { + continue; + } + return json_object_is_type(value, json_type_object); + } + return (0); +} + +static int cmor_set_nested_variable_entry(cmor_table_t *table, + char *variable_name, + char *brand_name, + json_object *json) +{ + extern int cmor_ntables; + int entry_len; + char variable_entry[CMOR_MAX_STRING]; + char *szTableId; + + szTableId = cmor_tables[cmor_ntables].szTable_id; + + if (brand_name[0] == '\0') { + entry_len = snprintf(variable_entry, CMOR_MAX_STRING, "%s", + variable_name); + } else { + entry_len = snprintf(variable_entry, CMOR_MAX_STRING, "%s_%s", + variable_name, brand_name); + } + + if (entry_len < 0 || entry_len >= CMOR_MAX_STRING) { + cmor_handle_error_variadic( + "Variable entry is too long for table: %s", + CMOR_CRITICAL, + szTableId); + cmor_ntables--; + return (1); + } + + return cmor_set_variable_entry(table, variable_entry, json); +} + /************************************************************************/ /* cmor_set_axis_entry() */ /************************************************************************/ @@ -1027,8 +1070,27 @@ int cmor_load_table_internal(char szTable[CMOR_MAX_STRING], int *table_id, if (attributes == NULL) { return (TABLE_ERROR); } - if (cmor_set_variable_entry(&cmor_tables[cmor_ntables], - varname, attributes) == 1) { + if (json_object_is_type(attributes, json_type_object) && + cmor_variable_entry_uses_nested_brands(attributes)) { + json_object_object_foreach(attributes, brandname, + brand_attributes) { + if (brandname[0] == '#') { + continue; + } + if (brand_attributes == NULL || + !json_object_is_type(brand_attributes, + json_type_object)) { + return (TABLE_ERROR); + } + if (cmor_set_nested_variable_entry( + &cmor_tables[cmor_ntables], varname, + brandname, brand_attributes) == 1) { + cmor_pop_traceback(); + return (TABLE_ERROR); + } + } + } else if (cmor_set_variable_entry(&cmor_tables[cmor_ntables], + varname, attributes) == 1) { cmor_pop_traceback(); return (TABLE_ERROR); } diff --git a/Test/test_cmor_CMIP7.py b/Test/test_cmor_CMIP7.py index 62229597..2180ef40 100644 --- a/Test/test_cmor_CMIP7.py +++ b/Test/test_cmor_CMIP7.py @@ -45,6 +45,7 @@ def setUp(self): Write out a simple file using CMOR """ self.input_json = Path("Test/input_cmip7.json") + self.nested_ocean_table = Path(CMIP7_TABLES_PATH) / "CMIP7_ocean_nested.json" # Set up CMOR cmor.setup(inpath=CMIP7_TABLES_PATH, netcdf_file_action=cmor.CMOR_REPLACE) @@ -58,8 +59,32 @@ def setUp(self): if error_flag: raise RuntimeError("CMOR dataset_json call failed") + self._write_nested_table( + Path("TestTables/CMIP7_ocean2d.json"), + self.nested_ocean_table, + ) + def tearDown(self): self.input_json.unlink(missing_ok=True) + self.nested_ocean_table.unlink(missing_ok=True) + + def _write_nested_table(self, source, destination): + with source.open() as table_handle: + table = json.load(table_handle) + + nested_entries = {} + for variable_entry, cfg in table["variable_entry"].items(): + out_name = cfg.get("out_name", variable_entry) + brand_name = "" + prefix = f"{out_name}_" + if variable_entry.startswith(prefix): + brand_name = variable_entry[len(prefix):] + nested_entries.setdefault(out_name, {})[brand_name] = cfg + + table["variable_entry"] = nested_entries + + with destination.open("w") as table_handle: + json.dump(table, table_handle, sort_keys=True, indent=4) def test_cmip7(self): data = [27] * (2 * 3 * 4) @@ -197,6 +222,57 @@ def test_secondary_modeling_realm(self): ds.close() + def test_nested_variable_entry(self): + data = [27] * (2 * 3 * 4) + tos = numpy.array(data) + tos.shape = (2, 3, 4) + lat = numpy.array([10, 20, 30]) + lat_bnds = numpy.array([5, 15, 25, 35]) + lon = numpy.array([0, 90, 180, 270]) + lon_bnds = numpy.array([-45, 45, + 135, + 225, + 315 + ]) + time = numpy.array([15.5, 45]) + time_bnds = numpy.array([0, 31, 60]) + cmor.load_table(self.nested_ocean_table.name) + cmorlat = cmor.axis("latitude", + coord_vals=lat, + cell_bounds=lat_bnds, + units="degrees_north") + cmorlon = cmor.axis("longitude", + coord_vals=lon, + cell_bounds=lon_bnds, + units="degrees_east") + cmortime = cmor.axis("time", + coord_vals=time, + cell_bounds=time_bnds, + units="days since 2018") + axes = [cmortime, cmorlat, cmorlon] + cmortos = cmor.variable("tos_tavg-u-hxy-sea", "degC", axes) + self.assertEqual(cmor.write(cmortos, tos), 0) + filename = cmor.close(cmortos, file_name=True) + self.assertEqual(cmor.close(), 0) + + ds = Dataset(filename) + attrs = ds.ncattrs() + test_attrs = { + 'branded_variable': 'tos_tavg-u-hxy-sea', + 'branding_suffix': 'tavg-u-hxy-sea', + 'temporal_label': 'tavg', + 'vertical_label': 'u', + 'horizontal_label': 'hxy', + 'area_label': 'sea', + 'realm': 'ocean', + } + + for attr, val in test_attrs.items(): + self.assertIn(attr, attrs) + self.assertEqual(val, ds.getncattr(attr)) + + ds.close() + if __name__ == '__main__': unittest.main() diff --git a/Test/test_python_branded_variable.py b/Test/test_python_branded_variable.py index 482b5de1..d57a4691 100644 --- a/Test/test_python_branded_variable.py +++ b/Test/test_python_branded_variable.py @@ -2,6 +2,7 @@ import cmor import unittest import os +from pathlib import Path from netCDF4 import Dataset @@ -46,19 +47,48 @@ def setUp(self): """ Write out a simple file using CMOR """ + self.input_json = Path("Test/input_branded_variable.json") + self.nested_table = Path("TestTables/CMIP6_Omon_branded_variable_nested.json") + # Set up CMOR cmor.setup(inpath="TestTables", netcdf_file_action=cmor.CMOR_REPLACE, logfile="cmor.log", create_subdirectories=0) # Define dataset using DATASET_INFO - with open("Test/input_branded_variable.json", "w") as input_file_handle: + with self.input_json.open("w") as input_file_handle: json.dump(DATASET_INFO, input_file_handle, sort_keys=True, indent=4) # read dataset info - error_flag = cmor.dataset_json("Test/input_branded_variable.json") + error_flag = cmor.dataset_json(str(self.input_json)) if error_flag: raise RuntimeError("CMOR dataset_json call failed") + self._write_nested_table( + Path("TestTables/CMIP6_Omon_branded_variable.json"), + self.nested_table, + ) + + def tearDown(self): + self.input_json.unlink(missing_ok=True) + self.nested_table.unlink(missing_ok=True) + + def _write_nested_table(self, source, destination): + with source.open() as table_handle: + table = json.load(table_handle) + + nested_entries = {} + for variable_entry, cfg in table["variable_entry"].items(): + out_name = cfg.get("out_name", variable_entry) + brand_name = "" + prefix = f"{out_name}_" + if variable_entry.startswith(prefix): + brand_name = variable_entry[len(prefix):] + nested_entries.setdefault(out_name, {})[brand_name] = cfg + + table["variable_entry"] = nested_entries + + with destination.open("w") as table_handle: + json.dump(table, table_handle, sort_keys=True, indent=4) def test_variable_without_branding_suffix(self): mip_table = "CMIP6_Omon_branded_variable.json" @@ -85,6 +115,30 @@ def test_variable_without_branding_suffix(self): ds.close() os.remove(filename) + def test_nested_variable_without_branding_suffix(self): + table_id = cmor.load_table(self.nested_table.name) + + itim = cmor.axis( + table_entry='time', + units='months since 2010-1-1', + coord_vals=[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], + cell_bounds=[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]) + ivar = cmor.variable('thetaoga', units='deg_C', axis_ids=[itim, ]) + + data = [280., ] * 12 + cmor.write(ivar, data) + filename = cmor.close(ivar, file_name=True) + + ds = Dataset(filename) + attrs = ds.ncattrs() + self.assertTrue('branding_suffix' not in attrs) + self.assertTrue('temporal_label' not in attrs) + self.assertTrue('vertical_label' not in attrs) + self.assertTrue('horizontal_label' not in attrs) + self.assertTrue('area_label' not in attrs) + ds.close() + os.remove(filename) + def test_variable_with_branding_suffix(self): mip_table = "CMIP6_Omon_branded_variable.json" @@ -116,6 +170,36 @@ def test_variable_with_branding_suffix(self): ds.close() os.remove(filename) + def test_nested_variable_with_branding_suffix(self): + table_id = cmor.load_table(self.nested_table.name) + + itim = cmor.axis( + table_entry='time', + units='months since 2010-1-1', + coord_vals=[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], + cell_bounds=[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]) + ivar = cmor.variable('thetaoga_w1-x2-y3-z4', units='deg_C', + axis_ids=[itim, ]) + + data = [280., ] * 12 + cmor.write(ivar, data) + filename = cmor.close(ivar, file_name=True) + + ds = Dataset(filename) + attrs = ds.ncattrs() + self.assertTrue('branding_suffix' in attrs) + self.assertEqual('w1-x2-y3-z4', ds.getncattr('branding_suffix')) + self.assertTrue('temporal_label' in attrs) + self.assertEqual('w1', ds.getncattr('temporal_label')) + self.assertTrue('vertical_label' in attrs) + self.assertEqual('x2', ds.getncattr('vertical_label')) + self.assertTrue('horizontal_label' in attrs) + self.assertEqual('y3', ds.getncattr('horizontal_label')) + self.assertTrue('area_label' in attrs) + self.assertEqual('z4', ds.getncattr('area_label')) + ds.close() + os.remove(filename) + if __name__ == '__main__': unittest.main() From 54dcf54b30bd82b881d431667d85c87be8e0934f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Apr 2026 22:28:50 +0000 Subject: [PATCH 2/3] fix: harden nested variable entry parsing Agent-Logs-Url: https://github.com/ilaflott/cmor/sessions/789e2979-03f5-480c-a8d7-6487d07b9c72 Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com> --- Src/cmor_tables.c | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/Src/cmor_tables.c b/Src/cmor_tables.c index e771854c..9b14ccce 100644 --- a/Src/cmor_tables.c +++ b/Src/cmor_tables.c @@ -285,15 +285,34 @@ int cmor_set_variable_entry(cmor_table_t * table, return (0); } -static int cmor_variable_entry_uses_nested_brands(json_object *json) +enum cmor_variable_entry_shape { + CMOR_VARIABLE_ENTRY_FLAT = 0, + CMOR_VARIABLE_ENTRY_NESTED = 1, + CMOR_VARIABLE_ENTRY_INVALID = -1 +}; + +static int cmor_get_variable_entry_shape(json_object *json) { + int has_objects = 0; + int has_non_objects = 0; + json_object_object_foreach(json, attr, value) { if (attr[0] == '#') { continue; } - return json_object_is_type(value, json_type_object); + if (json_object_is_type(value, json_type_object)) { + has_objects = 1; + } else { + has_non_objects = 1; + } + if (has_objects && has_non_objects) { + return (CMOR_VARIABLE_ENTRY_INVALID); + } } - return (0); + if (has_objects) { + return (CMOR_VARIABLE_ENTRY_NESTED); + } + return (CMOR_VARIABLE_ENTRY_FLAT); } static int cmor_set_nested_variable_entry(cmor_table_t *table, @@ -1063,6 +1082,7 @@ int cmor_load_table_internal(char szTable[CMOR_MAX_STRING], int *table_id, done = 1; } else if (strcmp(key, JSON_KEY_VARIABLE_ENTRY) == 0) { json_object_object_foreach(value, varname, attributes) { + int variable_entry_shape; if (varname[0] == '#') { continue; @@ -1070,8 +1090,17 @@ int cmor_load_table_internal(char szTable[CMOR_MAX_STRING], int *table_id, if (attributes == NULL) { return (TABLE_ERROR); } + variable_entry_shape = cmor_get_variable_entry_shape(attributes); + if (variable_entry_shape == CMOR_VARIABLE_ENTRY_INVALID) { + cmor_handle_error_variadic( + "Variable entry '%s' mixes nested brand objects with regular attributes", + CMOR_CRITICAL, + varname); + cmor_pop_traceback(); + return (TABLE_ERROR); + } if (json_object_is_type(attributes, json_type_object) && - cmor_variable_entry_uses_nested_brands(attributes)) { + variable_entry_shape == CMOR_VARIABLE_ENTRY_NESTED) { json_object_object_foreach(attributes, brandname, brand_attributes) { if (brandname[0] == '#') { @@ -1080,6 +1109,10 @@ int cmor_load_table_internal(char szTable[CMOR_MAX_STRING], int *table_id, if (brand_attributes == NULL || !json_object_is_type(brand_attributes, json_type_object)) { + cmor_handle_error_variadic( + "Nested variable entry '%s' brand '%s' must be an object", + CMOR_CRITICAL, + varname, brandname); return (TABLE_ERROR); } if (cmor_set_nested_variable_entry( From 33238618a5b56313205b9dfae80872d4e5d238fa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Apr 2026 22:30:18 +0000 Subject: [PATCH 3/3] refactor: tighten nested variable entry errors Agent-Logs-Url: https://github.com/ilaflott/cmor/sessions/789e2979-03f5-480c-a8d7-6487d07b9c72 Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com> --- Src/cmor_tables.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Src/cmor_tables.c b/Src/cmor_tables.c index 9b14ccce..a96a37bc 100644 --- a/Src/cmor_tables.c +++ b/Src/cmor_tables.c @@ -323,9 +323,6 @@ static int cmor_set_nested_variable_entry(cmor_table_t *table, extern int cmor_ntables; int entry_len; char variable_entry[CMOR_MAX_STRING]; - char *szTableId; - - szTableId = cmor_tables[cmor_ntables].szTable_id; if (brand_name[0] == '\0') { entry_len = snprintf(variable_entry, CMOR_MAX_STRING, "%s", @@ -339,8 +336,10 @@ static int cmor_set_nested_variable_entry(cmor_table_t *table, cmor_handle_error_variadic( "Variable entry is too long for table: %s", CMOR_CRITICAL, - szTableId); - cmor_ntables--; + table->szTable_id); + if (cmor_ntables >= 0 && table == &cmor_tables[cmor_ntables]) { + cmor_ntables--; + } return (1); }