From 1e47bc077f54d8b0a10e03b51357d323b6485498 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Wed, 25 Jan 2023 05:56:31 +0000 Subject: [PATCH 1/2] Fix a couple bugs: 1. Fix error message that appears when user specifies an invalid key in their config.yaml file. 2. In order to achieve the above neatly, I had to change the behavior of check_structure_dict in python_utils. Rather than simply printing the invalid key/value pair, it now returns a dictionary of invalid key/value pairs (that is empty if all keys are valid). I also fixed a minor bug here: even though this function *claimed* to detect all invalid entries, it actually only printed the first before returning. Now all invalid entries are returned. --- ush/python_utils/config_parser.py | 21 ++++++++++++--------- ush/setup.py | 16 +++++++--------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/ush/python_utils/config_parser.py b/ush/python_utils/config_parser.py index c09ff8c9c5..0a968ec6cd 100644 --- a/ush/python_utils/config_parser.py +++ b/ush/python_utils/config_parser.py @@ -465,25 +465,26 @@ def update_dict(dict_o, dict_t, provide_default=False): def check_structure_dict(dict_o, dict_t): """Check if a dictionary's structure follows a template. - The invalid entries are printed to the screen. + The invalid entries are returned as a list of lists. + If all entries are valid, returns an empty dictionary Args: dict_o: target dictionary dict_t: template dictionary to compare structure to Returns: - Boolean + inval: dictionary of invalid key-value pairs """ + inval = {} for k, v in dict_o.items(): if k in dict_t.keys(): v1 = dict_t[k] if isinstance(v, dict) and isinstance(v1, dict): r = check_structure_dict(v, v1) - if not r: - return False + if r: + inval.update(r) else: - print(f"INVALID ENTRY: {k}={v}") - return False - return True + inval[k] = v + return inval def filter_dict(dict_o, keys_regex): @@ -579,9 +580,11 @@ def cfg_main(): cfg_t = load_config_file(args.validate, 1) r = check_structure_dict(cfg, cfg_t) if r: - print("SUCCESS") - else: + for k in r: + print(f"INVALID ENTRY: {k}={r[k]}") print("FAILURE") + else: + print("SUCCESS") else: if args.template: cfg = flatten_dict(cfg) diff --git a/ush/setup.py b/ush/setup.py index c2fdc0affd..a761990886 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -80,15 +80,13 @@ def load_config_for_setup(ushdir, default_config, user_config): # Make sure the keys in user config match those in the default # config. - if not check_structure_dict(cfg_u, cfg_d): - raise Exception( - dedent( - f""" - User-specified variable "{key}" in {user_config} is not valid - Check {EXPT_DEFAULT_CONFIG_FN} for allowed user-specified variables\n - """ - ) - ) + invalid = check_structure_dict(cfg_u, cfg_d) + if invalid: + errmsg = "Invalid key(s) specified in {user_config}:\n" + for entry in invalid: + errmsg = errmsg + f"{entry} = {invalid[entry]}\n" + errmsg = errmsg + f"\nCheck {default_config} for allowed user-specified variables\n" + raise Exception(errmsg) # Mandatory variables *must* be set in the user's config; the default value is invalid mandatory = ["user.MACHINE"] From 90d283d1fe6686cff3092a594d829d64cec0d887 Mon Sep 17 00:00:00 2001 From: Michael Kavulich Date: Fri, 27 Jan 2023 14:17:58 -0700 Subject: [PATCH 2/2] Update ush/python_utils/config_parser.py Suggestions for correct docstring style from @zmoon Co-authored-by: Zachary Moon --- ush/python_utils/config_parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ush/python_utils/config_parser.py b/ush/python_utils/config_parser.py index 0a968ec6cd..6510af62eb 100644 --- a/ush/python_utils/config_parser.py +++ b/ush/python_utils/config_parser.py @@ -472,7 +472,7 @@ def check_structure_dict(dict_o, dict_t): dict_o: target dictionary dict_t: template dictionary to compare structure to Returns: - inval: dictionary of invalid key-value pairs + dict: Invalid key-value pairs. """ inval = {} for k, v in dict_o.items():