diff --git a/src/fmu/dataio/dataio.py b/src/fmu/dataio/dataio.py index 71c2a14b2..6e740bb22 100644 --- a/src/fmu/dataio/dataio.py +++ b/src/fmu/dataio/dataio.py @@ -89,6 +89,10 @@ def _check_global_config( Currently far from a full validation. For now, just check that some required keys are present in the config and warn/raise if not. + + PS! Seems like a good job for jsonschema, but the produced error message are not + informative enough to provide meaningful information to user when something is + wrong. """ if not globalconfig and not strict: @@ -97,20 +101,73 @@ def _check_global_config( ) return False - # strict is True: - config_required_keys = ["access", "masterdata", "model"] + msg = "" missing_keys = [] + + # check required key presence + config_required_keys = ["access", "masterdata", "model"] for required_key in config_required_keys: if required_key not in globalconfig: missing_keys.append(required_key) if missing_keys: - msg = ( + msg += ( "One or more keys required for valid metadata are not found: " f"{missing_keys} (perhaps the config is empty?) " ) + + # check "stratigraphy" + if "stratigraphy" in globalconfig: + # we currently allow (why?) stratigraphy key missing from config. + strat = globalconfig["stratigraphy"] + if not isinstance(strat, dict): + msg += "The 'stratigraphy' must be a dictionary.\n" + + # Loop the entries in 'stratigraphy' + # These keys are custom, but we want error messages to point to the key when + # issues are discovered. This makes it tricky to use jsonschema. Pydantic might + # be an option. But for now, just go through all items and do defined checks. + + for key, item in strat.items(): + if "name" not in item: + msg += f"stratigraphy.{key}: 'name' is missing. \n" + elif not isinstance(item["name"], str): + msg += f"stratigraphy.{key}: 'name' must be a string.\n" + + if "stratigraphic" not in item: + msg += f"stratigraphy.{key}: 'stratigraphic' is missing.\n" + elif not isinstance(item["stratigraphic"], bool): + msg += f"stratigraphy.{key}: 'stratigraphic' must be a boolean.\n" + + if "alias" in item: + if not isinstance(item["alias"], list): + msg += f"stratigraphy.{key}: 'alias' must be a list.\n" + else: + for alias in item["alias"]: + if not isinstance(alias, str): + msg += ( + f"stratigraphy.{key}: 'alias' items must be strings\n" + ) + + # After checking and warning, remove empty entries + item["alias"] = list(filter(lambda i: i is not None, item["alias"])) + + if "stratigraphic_alias" in item: + if not isinstance(item["stratigraphic_alias"], list): + msg += f"stratigraphy.{key}: 'stratigraphic_alias' must be list.\n" + else: + for alias in item["stratigraphic_alias"]: + if not isinstance(alias, str): + msg += f"stratigraphy.{key}: 'stratigraphic_alias' items " + msg += "must be strings.\n" + + # After checking and warning, remove empty entries + item["stratigraphic_alias"] = list( + filter(lambda i: i is not None, item["stratigraphic_alias"]) + ) + + if msg: if "err" in action: - msg = msg + " STOP!" raise ValueError(msg) else: msg += ( diff --git a/tests/test_units/test_dataio.py b/tests/test_units/test_dataio.py index 6c1597146..e4bb40026 100644 --- a/tests/test_units/test_dataio.py +++ b/tests/test_units/test_dataio.py @@ -4,6 +4,8 @@ import pathlib import sys +from copy import deepcopy + import pytest import yaml @@ -77,6 +79,62 @@ def test_config_miss_required_fields(globalconfig1, regsurf): read_metadata(out) +def test_config_stratigraphy_empty_entries_alias(globalconfig2, regsurf): + """Test that empty entries in 'alias' is detected and warned and removed.""" + cfg = deepcopy(globalconfig2) + cfg["stratigraphy"]["TopVolantis"]["alias"] += [None] + + with pytest.warns(PendingDeprecationWarning, match="'alias' items must be strings"): + exp = ExportData(config=cfg, content="depth", name="TopVolantis") + metadata = exp.generate_metadata(regsurf) + + assert None not in metadata["data"]["alias"] + + +def test_config_stratigraphy_empty_entries_stratigraphic_alias(globalconfig2, regsurf): + """Test that empty entries in 'stratigraphic_alias' detected and warned.""" + + # Note! stratigraphic_alias is not implemented, but we still check consistency + + cfg = deepcopy(globalconfig2) + cfg["stratigraphy"]["TopVolantis"]["stratigraphic_alias"] += [None] + + with pytest.warns( + PendingDeprecationWarning, match="'stratigraphic_alias' items must be strings" + ): + ExportData(config=cfg, content="depth") + + +def test_config_stratigraphy_empty_name(globalconfig2): + """Test that empty 'name' is detected and warned.""" + cfg = deepcopy(globalconfig2) + cfg["stratigraphy"]["TopVolantis"]["name"] = None + + with pytest.warns( + PendingDeprecationWarning, + match="stratigraphy.TopVolantis: 'name' must be a string", + ): + ExportData(config=cfg, content="depth") + + +def test_config_stratigraphy_stratigraphic_not_bool(globalconfig2): + """Test that non-boolean 'stratigraphic' is detected and warned.""" + cfg = deepcopy(globalconfig2) + cfg["stratigraphy"]["TopVolantis"]["stratigraphic"] = None + + with pytest.warns( + PendingDeprecationWarning, match="'stratigraphic' must be a bool" + ): + ExportData(config=cfg, content="depth") + + cfg["stratigraphy"]["TopVolantis"]["stratigraphic"] = "a string" + + with pytest.warns( + PendingDeprecationWarning, match="'stratigraphic' must be a bool" + ): + ExportData(config=cfg, content="depth") + + def test_update_check_settings_shall_fail(globalconfig1): # pylint: disable=unexpected-keyword-arg with pytest.raises(TypeError):