Skip to content

Commit ce4b8aa

Browse files
authored
Fix list append nested in subclass not working (#710)
1 parent 172f9e1 commit ce4b8aa

File tree

6 files changed

+60
-20
lines changed

6 files changed

+60
-20
lines changed

CHANGELOG.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ Fixed
3838
<https://github.com/omni-us/jsonargparse/pull/707>`__).
3939
- Nested generic dataclasses not working correctly (`#709
4040
<https://github.com/omni-us/jsonargparse/pull/709>`__).
41+
- List append nested in subclass not working (`#710
42+
<https://github.com/omni-us/jsonargparse/pull/710>`__).
4143

4244

4345
v4.38.0 (2025-03-26)

jsonargparse/_actions.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def _find_action_and_subcommand(
6464
actions = [a for a in actions if not isinstance(a, exclude)]
6565
fallback_action = None
6666
for action in actions:
67-
if action.dest == dest:
67+
if action.dest == dest or f"--{dest}" in action.option_strings:
6868
if isinstance(action, _ActionConfigLoad):
6969
fallback_action = action
7070
else:

jsonargparse/_core.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -953,6 +953,9 @@ def save_paths(cfg):
953953
## Methods related to defaults ##
954954

955955
def _get_default_config_files(self) -> List[Tuple[Optional[str], Path]]:
956+
if getattr(self, "_inner_parser", False):
957+
return []
958+
956959
default_config_files = []
957960

958961
for key, parser in parent_parsers.get():
@@ -1022,7 +1025,7 @@ def get_defaults(self, skip_validation: bool = False, **kwargs) -> Namespace:
10221025
if not default_config_file_content.strip():
10231026
continue
10241027
with change_to_path_dir(default_config_file), parser_context(parent_parser=self):
1025-
cfg_file = self._load_config_parser_mode(default_config_file.get_content(), key=key)
1028+
cfg_file = self._load_config_parser_mode(default_config_file_content, key=key, prev_cfg=cfg)
10261029
cfg = self.merge_config(cfg_file, cfg)
10271030
try:
10281031
with _ActionPrintConfig.skip_print_config():
@@ -1365,11 +1368,15 @@ def _apply_actions(
13651368
continue
13661369

13671370
action_dest = action.dest if subcommand is None else subcommand + "." + action.dest
1371+
append = False
1372+
if action_dest not in cfg and key.endswith("+"):
1373+
append = True
1374+
cfg[action_dest] = cfg.pop(key)
13681375
value = cfg[action_dest]
13691376
if skip_fn and skip_fn(value):
13701377
continue
13711378
with parser_context(parent_parser=self, lenient_check=True):
1372-
value = self._check_value_key(action, value, action_dest, prev_cfg)
1379+
value = self._check_value_key(action, value, action_dest, prev_cfg, append=append)
13731380
if isinstance(action, _ActionConfigLoad):
13741381
config_keys.add(action_dest)
13751382
keys.append(action_dest)
@@ -1393,10 +1400,11 @@ def merge_config(self, cfg_from: Namespace, cfg_to: Namespace) -> Namespace:
13931400
with parser_context(parent_parser=self):
13941401
ActionTypeHint.discard_init_args_on_class_path_change(self, cfg_to, cfg_from)
13951402
cfg_to.update(cfg_from)
1396-
ActionTypeHint.apply_appends(self, cfg_to)
13971403
return cfg_to
13981404

1399-
def _check_value_key(self, action: argparse.Action, value: Any, key: str, cfg: Optional[Namespace]) -> Any:
1405+
def _check_value_key(
1406+
self, action: argparse.Action, value: Any, key: str, cfg: Optional[Namespace], append: bool = False
1407+
) -> Any:
14001408
"""Checks the value for a given action.
14011409
14021410
Args:
@@ -1421,7 +1429,7 @@ def _check_value_key(self, action: argparse.Action, value: Any, key: str, cfg: O
14211429
value = action.check_type(value, self)
14221430
elif hasattr(action, "_check_type"):
14231431
with parser_context(parent_parser=self):
1424-
value = action._check_type_(value, cfg=cfg) # type: ignore[attr-defined]
1432+
value = action._check_type_(value, cfg=cfg, append=append) # type: ignore[attr-defined]
14251433
elif action.type is not None:
14261434
try:
14271435
if action.nargs in {None, "?"} or action.nargs == 0:

jsonargparse/_typehints.py

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
_find_parent_action,
4444
_is_action_value_list,
4545
parent_parsers_context,
46+
parse_kwargs,
4647
remove_actions,
4748
)
4849
from ._common import (
@@ -484,16 +485,6 @@ def supports_append(action):
484485
)
485486
)
486487

487-
@staticmethod
488-
def apply_appends(parser, cfg):
489-
for key in [k for k in cfg.keys() if k.endswith("+")]:
490-
action = _find_action(parser, key[:-1])
491-
if ActionTypeHint.supports_append(action):
492-
with parser_context(load_value_mode=parser.parser_mode):
493-
val = action._check_type_(cfg[key], append=True, cfg=cfg)
494-
cfg[key[:-1]] = val
495-
cfg.pop(key)
496-
497488
def serialize(self, value, dump_kwargs=None):
498489
sub_add_kwargs = getattr(self, "sub_add_kwargs", {})
499490
with dump_kwargs_context(dump_kwargs):
@@ -1058,10 +1049,14 @@ def adapt_typehints(
10581049
if serialize and isinstance(val, str):
10591050
return val
10601051

1061-
val_input = val
1052+
prev_implicit_defaults = False
10621053
if prev_val is None and not inspect.isabstract(typehint) and not is_protocol(typehint):
10631054
with suppress(ValueError):
10641055
prev_val = Namespace(class_path=get_import_path(typehint)) # implicit class_path
1056+
if parse_kwargs.get().get("defaults") is True:
1057+
prev_implicit_defaults = True
1058+
1059+
val_input = val
10651060
val = subclass_spec_as_namespace(val, prev_val)
10661061
if not is_subclass_spec(val):
10671062
msg = "Does not implement protocol" if is_protocol(typehint) else "Not a valid subclass of"
@@ -1088,6 +1083,9 @@ def adapt_typehints(
10881083
return_type = get_return_type(val_class, logger)
10891084
if is_subclass_or_implements_protocol(return_type, typehint):
10901085
not_subclass = False
1086+
elif prev_implicit_defaults:
1087+
inner_parser = ActionTypeHint.get_class_parser(typehint, sub_add_kwargs)
1088+
prev_val.init_args = inner_parser.get_defaults()
10911089
if not_subclass:
10921090
msg = "implement protocol" if is_protocol(typehint) else "correspond to a subclass of"
10931091
raise_unexpected_value(f"Import path {val['class_path']} does not {msg} {typehint.__name__}")

jsonargparse_tests/test_subclasses.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,7 +1080,9 @@ def test_subclass_discard_init_args_with_default_config_files(parser, tmp_cwd, l
10801080
cfg = parser.parse_args(['--cal={"class_path": "calendar.Calendar", "init_args": {"firstweekday": 3}}'])
10811081
assert "discarding init_args: {'param': '1'}" in logs.getvalue()
10821082
assert cfg.cal.init_args == Namespace(firstweekday=3)
1083-
assert type(parser.instantiate_classes(cfg).cal) is Calendar
1083+
with capture_logs(logger) as logs:
1084+
assert type(parser.instantiate_classes(cfg).cal) is Calendar
1085+
assert logs.getvalue()
10841086

10851087

10861088
class Arch:
@@ -1165,7 +1167,9 @@ def test_discard_init_args_config_nested(parser, logger, tmp_cwd, method):
11651167
with capture_logs(logger) as logs:
11661168
cfg = parser.parse_args([f"--cfg={config_path}"])
11671169
assert "discarding init_args: {'s1': 'x'}" in logs.getvalue()
1168-
init = parser.instantiate_classes(cfg)
1170+
with capture_logs(logger) as logs:
1171+
init = parser.instantiate_classes(cfg)
1172+
assert logs.getvalue()
11691173
assert isinstance(init.main, ConfigDiscardMain)
11701174
assert isinstance(init.main.sub, ConfigDiscardSub2)
11711175

@@ -1216,7 +1220,9 @@ def test_subclass_discard_init_args_dict_looks_like_subclass(parser, logger, tmp
12161220
with capture_logs(logger) as logs:
12171221
cfg = parser.parse_args([f"--cfg={config_paths[1]}", f"--cfg={config_paths[2]}"])
12181222
assert "discarding init_args: {'s1': 1}" in logs.getvalue()
1219-
init = parser.instantiate_classes(cfg)
1223+
with capture_logs(logger) as logs:
1224+
init = parser.instantiate_classes(cfg)
1225+
assert logs.getvalue()
12201226
assert isinstance(init.main, DictDiscardMain)
12211227
assert isinstance(init.main.sub, dict)
12221228
assert init.main.sub["init_args"]["s2"] == 2

jsonargparse_tests/test_typehints.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,32 @@ def test_list_append_subcommand_subparser_default_config_files(parser, subparser
499499
assert cfg.sub.nums == [2]
500500

501501

502+
class NestedList:
503+
def __init__(
504+
self,
505+
nested: List[Dict[str, str]] = [{"a": "random_crop"}, {"b": "random_blur"}],
506+
) -> None:
507+
self.nested = nested
508+
509+
510+
def test_list_append_dicts_nested_with_default(parser):
511+
parser.add_argument("--cls", type=NestedList, default={"class_path": "NestedList"})
512+
cfg = parser.parse_args(['--cls.nested+={"d":"random_perspective"}'])
513+
assert cfg.cls.class_path == f"{__name__}.NestedList"
514+
assert cfg.cls.init_args == Namespace(
515+
nested=[{"a": "random_crop"}, {"b": "random_blur"}, {"d": "random_perspective"}]
516+
)
517+
518+
519+
def test_list_append_dicts_nested_without_default(parser):
520+
parser.add_argument("--cls", type=NestedList)
521+
cfg = parser.parse_args(["--cls=NestedList", '--cls.nested+={"d":"random_perspective"}'])
522+
assert cfg.cls.class_path == f"{__name__}.NestedList"
523+
assert cfg.cls.init_args == Namespace(
524+
nested=[{"a": "random_crop"}, {"b": "random_blur"}, {"d": "random_perspective"}]
525+
)
526+
527+
502528
# dict tests
503529

504530

0 commit comments

Comments
 (0)