Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nested values not validated when type not subclass and nested keys for subclass #506

Merged
merged 2 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,15 @@ Fixed
- ``format_usage()`` not working (`#501
<https://github.com/omni-us/jsonargparse/issues/501>`__).
- Not able to modify init args for callable with class return and default class
(`#5?? <https://github.com/omni-us/jsonargparse/pull/5??>`__).
(`#504 <https://github.com/omni-us/jsonargparse/pull/504>`__).
- Nested values not validated when type not subclass and nested keys for
subclass (`#503 comment
<https://github.com/omni-us/jsonargparse/issues/503#issuecomment-2119724341>`__).

Changed
^^^^^^^
- When parsing fails due to unexpected key, now there are specific error
messages for the cases of groups and subcommands.


v4.28.0 (2024-04-17)
Expand Down
36 changes: 20 additions & 16 deletions jsonargparse/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
_ActionSubCommands,
_find_action,
_find_action_and_subcommand,
_find_parent_action,
_find_parent_action_and_subcommand,
_is_action_value_list,
_is_branch_key,
Expand Down Expand Up @@ -67,6 +66,7 @@
recreate_branches,
split_key,
split_key_leaf,
split_key_root,
strip_meta,
)
from ._optionals import (
Expand Down Expand Up @@ -1050,20 +1050,19 @@ def check_required(cfg, parser, prefix=""):
check_required(cfg.get(subcommand), subparser, subcommand + ".")

def check_values(cfg):
for key in cfg.get_sorted_keys():
val = cfg[key]
action = _find_action(self, key)
sorted_keys = {k: _find_action(self, k) for k in cfg.get_sorted_keys()}
for key, action in sorted_keys.items():
parent_action = None
if action is None:
if (
_is_branch_key(self, key)
or key.endswith(".class_path")
or key.endswith(".dict_kwargs")
or ".init_args" in key
):
continue
action = _find_parent_action(self, key, exclude=_ActionConfigLoad)
if action and not ActionTypeHint.is_subclass_typehint(action):
if _is_branch_key(self, key):
continue
parent_action, subcommand = _find_parent_action_and_subcommand(self, key, exclude=_ActionConfigLoad)
if parent_action:
parent_key = subcommand + "." + parent_action.dest if subcommand else parent_action.dest
if key.startswith(parent_key + ".") and sorted_keys.get(parent_key) is parent_action:
# only check action once with entire value
continue
val = cfg[key]
if action is not None:
if val is None and skip_none:
continue
Expand All @@ -1074,10 +1073,15 @@ def check_values(cfg):
val == {} and ActionTypeHint.is_subclass_typehint(action) and key not in self.required_args
):
raise ex
elif key in self.groups and hasattr(self.groups[key], "instantiate_class"):
raise TypeError(f"Class group {key!r} got an unexpected value: {val}.")
else:
raise NSKeyError(f'No action for key "{key}" to check its value.')
if isinstance(parent_action, _ActionSubCommands) and "." in key:
subcommand, subkey = split_key_root(key)
raise NSKeyError(f"Subcommand '{subcommand}' does not accept nested key '{subkey}'")
group_key = next((g for g in self.groups if key.startswith(g + ".")), None)
if group_key:
subkey = key[len(group_key) + 1 :]
raise NSKeyError(f"Group '{group_key}' does not accept nested key '{subkey}'")
raise NSKeyError(f"Key '{key}' is not expected")

try:
if not skip_required and not lenient_check.get():
Expand Down
10 changes: 10 additions & 0 deletions jsonargparse_tests/test_dataclass_like.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,16 @@ def test_add_argument_dataclass_type(parser):
assert isinstance(init.b.b2, DataClassA)


def test_add_argument_dataclass_unexpected_keys(parser):
parser.add_argument("--b", type=DataClassB)
invalid = {
"class_path": f"{__name__}.DataClassB",
}
with pytest.raises(ArgumentError) as ctx:
parser.parse_args([f"--b={invalid}"])
ctx.match("Group 'b' does not accept nested key 'class_path'")


@dataclasses.dataclass
class DataRequiredAttr:
a1: str
Expand Down
2 changes: 1 addition & 1 deletion jsonargparse_tests/test_signatures.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ def test_add_class_without_parameters(parser):
config = {"no_params": {"class_path": f"{__name__}.NoParams"}}
with pytest.raises(ArgumentError) as ctx:
parser.parse_args([f"--cfg={config}"])
ctx.match("'no_params' got an unexpected value")
ctx.match("Group 'no_params' does not accept nested key 'class_path'")


class NestedWithParams:
Expand Down
8 changes: 4 additions & 4 deletions jsonargparse_tests/test_subclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ def test_subclass_class_name_then_invalid_init_args(parser):
parser.add_argument("--op", type=Union[Calendar, GzipFile])
with pytest.raises(ArgumentError) as ctx:
parser.parse_args(["--op=TextCalendar", "--op=GzipFile", "--op.firstweekday=2"])
ctx.match('No action for key "firstweekday"')
ctx.match("Key 'firstweekday' is not expected")


# dict parameter tests
Expand Down Expand Up @@ -1360,7 +1360,7 @@ def test_subclass_print_config(parser):
assert obtained == {"a1": {"class_path": "calendar.Calendar", "init_args": {"firstweekday": 0}}, "a2": 7}

err = get_parse_args_stderr(parser, ["--g.a1=calendar.Calendar", "--g.a1.invalid=1", "--print_config"])
assert 'No action for key "invalid"' in err
assert "Key 'invalid' is not expected" in err


class PrintConfigRequired:
Expand Down Expand Up @@ -1438,7 +1438,7 @@ def test_subclass_error_unexpected_init_arg(parser):
init_args = '"init_args": {"unexpected_arg": True}'
with pytest.raises(ArgumentError) as ctx:
parser.parse_args(["--op={" + class_path + ", " + init_args + "}"])
ctx.match('No action for key "unexpected_arg"')
ctx.match("Key 'unexpected_arg' is not expected")


def test_subclass_invalid_class_path_value(parser):
Expand Down Expand Up @@ -1507,7 +1507,7 @@ def test_subclass_implicit_class_path(parser):
assert cfg.implicit.init_args == Namespace(a=3, b="")
with pytest.raises(ArgumentError) as ctx:
parser.parse_args(['--implicit={"c": null}'])
ctx.match('No action for key "c" to check its value')
ctx.match("Key 'c' is not expected")


# error messages tests
Expand Down
2 changes: 1 addition & 1 deletion jsonargparse_tests/test_subcommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def test_subcommands_parse_string_implicit_subcommand(subcommands_parser):
assert cfg["a"] == {"ap1": "ap1_cfg", "ao1": "ao1_def"}
with pytest.raises(ArgumentError) as ctx:
subcommands_parser.parse_string('{"a": {"ap1": "ap1_cfg", "unk": "unk_cfg"}}')
ctx.match('No action for key "unk"')
ctx.match("Subcommand 'a' does not accept nested key 'unk'")


def test_subcommands_parse_string_first_implicit_subcommand(subcommands_parser):
Expand Down