Skip to content

Commit

Permalink
Logging improvements in chgres_cube namelist creation (ufs-community#661
Browse files Browse the repository at this point in the history
)

* WIP

* WIP

* Improved chgres_cube logging & dot delimited key paths

* Rocoto notebook test update

* Add additional key as context for chgres_cube logging

* Apply suggestions from code review

Co-authored-by: Paul Madden <[email protected]>

* Change var name & DRY out repeated lines

* Add update_input_files() parameters

---------

Co-authored-by: Paul Madden <[email protected]>
  • Loading branch information
Byrnetp and maddenp-noaa authored Nov 27, 2024
1 parent b9f70c1 commit b253a6d
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 25 deletions.
4 changes: 2 additions & 2 deletions notebooks/tests/test_rocoto.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ def test_building_simple_workflow():
assert tb.cell_output_text(11) == err_yaml
err_out = (
"ERROR 3 UW schema-validation errors found",
"ERROR Error at workflow -> attrs:",
"ERROR Error at workflow.attrs:",
"ERROR 'realtime' is a required property",
"ERROR Error at workflow -> tasks -> task_greet:",
"ERROR Error at workflow.tasks.task_greet:",
"ERROR 'command' is a required property",
"ERROR Error at workflow:",
"ERROR 'log' is a required property",
Expand Down
2 changes: 1 addition & 1 deletion src/uwtools/config/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def walk_key_path(config: dict, key_path: list[str]) -> tuple[dict, str]:
pathstr = "<unknown>"
for key in key_path:
keys.append(key)
pathstr = " -> ".join(keys)
pathstr = ".".join(keys)
try:
subconfig = config[key]
except KeyError as e:
Expand Down
2 changes: 1 addition & 1 deletion src/uwtools/config/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def validate(schema: dict, desc: str, config: dict) -> bool:
log_msg = "%s UW schema-validation error%s found in %s"
log_method(log_msg, len(errors), "" if len(errors) == 1 else "s", desc)
for error in errors:
log.error("Error at %s:", " -> ".join(str(k) for k in error.path))
log.error("Error at %s:", ".".join(str(k) for k in error.path))
log.error("%s%s", INDENT, error.message)
return not bool(errors)

Expand Down
29 changes: 16 additions & 13 deletions src/uwtools/drivers/chgres_cube.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,29 @@ def namelist_file(self):
"""
The namelist file.
"""

def update_input_files(k, config_files, input_files):
v = config_files[k]
context = ".".join(["config", k])
if isinstance(v, str):
input_files.append((grid_path / v, context))
else:
input_files.extend([(grid_path / f, context) for f in v])

fn = "fort.41"
yield self.taskname(f"namelist file {fn}")
path = self.rundir / fn
yield asset(path, path.is_file)
input_files = []
namelist = self.config[STR.namelist]
if base_file := namelist.get(STR.basefile):
input_files.append(base_file)
context = ".".join([STR.namelist, STR.basefile])
input_files.append((base_file, context))
if update_values := namelist.get(STR.updatevalues):
config_files = update_values["config"]
for k in ["mosaic_file_target_grid", "varmap_file", "vcoord_file_target_grid"]:
input_files.append(config_files[k])
context = ".".join(["config", k])
input_files.append((config_files[k], context))
for k in [
"atm_core_files_input_grid",
"atm_files_input_grid",
Expand All @@ -47,23 +58,15 @@ def namelist_file(self):
]:
if k in config_files:
grid_path = Path(config_files["data_dir_input_grid"])
v = config_files[k]
if isinstance(v, str):
input_files.append(grid_path / v)
else:
input_files.extend([grid_path / f for f in v])
update_input_files(k, config_files, input_files)
for k in [
"orog_files_input_grid",
"orog_files_target_grid",
]:
if k in config_files:
grid_path = Path(config_files[k.replace("files", "dir")])
v = config_files[k]
if isinstance(v, str):
input_files.append(grid_path / v)
else:
input_files.extend([grid_path / f for f in v])
yield [file(Path(input_file)) for input_file in input_files]
update_input_files(k, config_files, input_files)
yield [file(Path(input_file), context) for input_file, context in input_files]
self._create_user_updated_config(
config_class=NMLConfig,
config_values=namelist,
Expand Down
2 changes: 1 addition & 1 deletion src/uwtools/fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def _set_config_block(self) -> None:
for key in self._keys:
nav.append(key)
if key not in cfg:
raise UWConfigError("Failed following YAML key(s): %s" % " -> ".join(nav))
raise UWConfigError("Failed following YAML key(s): %s" % ".".join(nav))
log.debug("Following config key '%s'", key)
cfg = cfg[key]
if not isinstance(cfg, dict):
Expand Down
6 changes: 3 additions & 3 deletions src/uwtools/tests/config/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -556,17 +556,17 @@ def test_realize_config_values_needed_yaml(caplog):
def test_walk_key_path_fail_bad_key_path():
with raises(UWError) as e:
tools.walk_key_path({"a": {"b": {"c": "cherry"}}}, ["a", "x"])
assert str(e.value) == "Bad config path: a -> x"
assert str(e.value) == "Bad config path: a.x"


def test_walk_key_path_fail_bad_leaf_value():
with raises(UWError) as e:
tools.walk_key_path({"a": {"b": {"c": "cherry"}}}, ["a", "b", "c"])
assert str(e.value) == "Value at a -> b -> c must be a dictionary"
assert str(e.value) == "Value at a.b.c must be a dictionary"


def test_walk_key_path_pass():
expected = ({"c": "cherry"}, "a -> b")
expected = ({"c": "cherry"}, "a.b")
assert tools.walk_key_path({"a": {"b": {"c": "cherry"}}}, ["a", "b"]) == expected


Expand Down
4 changes: 3 additions & 1 deletion src/uwtools/tests/drivers/test_chgres_cube.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ def test_ChgresCube_namelist_file_missing_base_file(caplog, driverobj):
driverobj._config["namelist"]["base_file"] = base_file
path = Path(refs(driverobj.namelist_file()))
assert not path.exists()
assert regex_logged(caplog, "missing.nml: State: Not Ready (external asset)")
assert regex_logged(
caplog, "missing.nml (namelist.base_file): State: Not Ready (external asset)"
)


def test_ChgresCube_output(driverobj):
Expand Down
2 changes: 1 addition & 1 deletion src/uwtools/tests/test_fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def test_Stager__config_block_fail_bad_key_path(assets, source):
config = cfgdict if source == "dict" else cfgfile
with raises(UWConfigError) as e:
ConcreteStager(target_dir=dstdir, config=config, keys=["a", "x"])
assert str(e.value) == "Failed following YAML key(s): a -> x"
assert str(e.value) == "Failed following YAML key(s): a.x"


@mark.parametrize("val", [None, True, False, "str", 42, 3.14, [], tuple()])
Expand Down
6 changes: 4 additions & 2 deletions src/uwtools/utils/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ def existing(path: Path):


@external
def file(path: Path):
def file(path: Path, context: str = ""):
"""
An existing file or symlink to an existing file.
:param path: Path to the file.
:param context: Optional additional context for the file.
"""
yield "File %s" % path
suffix = f" ({context})" if context else ""
yield "File %s%s" % (path, suffix)
yield asset(path, path.is_file)


Expand Down

0 comments on commit b253a6d

Please sign in to comment.