Fixed append to list not working for default_config_files #157.

Author: mauvilsa

CHANGELOG.rst

@@ -22,6 +22,8 @@ Fixed
 - Failure to parse when subcommand has no options `#158
   <https://github.com/omni-us/jsonargparse/issues/158>`__.
 - Optional packages being imported even though not used.
+- Append to list not working for ``default_config_files`` `#157
+  <https://github.com/omni-us/jsonargparse/issues/157>`__.
 
 
 v4.13.2 (2022-08-31)

jsonargparse/actions.py

@@ -175,7 +175,7 @@ def __call__(self, parser, cfg, values, option_string=None):
     @staticmethod
     def apply_config(parser, cfg, dest, value) -> None:
         with _ActionSubCommands.not_single_subcommand(), previous_config_context(cfg):
-            if dest not in cfg:
+            if cfg.get(dest) is None:
                 cfg[dest] = []
             kwargs = {'env': False, 'defaults': False, '_skip_check': True, '_fail_no_subcommand': False}
             try:
@@ -192,6 +192,8 @@ def apply_config(parser, cfg, dest, value) -> None:
                 cfg_file = parser.parse_path(value, **kwargs)
             cfg[dest].append(cfg_path)
             cfg.update(cfg_file)
+            from .typehints import ActionTypeHint
+            ActionTypeHint.apply_appends(parser, cfg)
 
 
 previous_config: ContextVar = ContextVar('previous_config', default=None)

jsonargparse/core.py

@@ -259,7 +259,6 @@ def _parse_common(
         skip_required: bool = False,
         skip_subcommands: bool = False,
         fail_no_subcommand: bool = True,
-        cfg_base: Optional[Namespace] = None,
         log_message: Optional[str] = None,
     ) -> Namespace:
         """Common parsing code used by other parse methods.
@@ -273,7 +272,6 @@ def _parse_common(
             skip_required: Whether to skip check of required arguments.
             skip_subcommands: Whether to skip subcommand processing.
             fail_no_subcommand: Whether to fail if no subcommand given.
-            cfg_base: A base configuration object.
             log_message: Message to log at INFO level after parsing.
 
         Returns:
@@ -285,18 +283,7 @@ def _parse_common(
         if not skip_subcommands:
             _ActionSubCommands.handle_subcommands(self, cfg, env=env, defaults=defaults, fail_no_subcommand=fail_no_subcommand)
 
-        if cfg_base is not None:
-            cfg = self.merge_config(cfg, cfg_base)
-
-        if env:
-            with _ActionPrintConfig.skip_print_config():
-                cfg_env = self.parse_env(defaults=defaults, _skip_check=True, _skip_subcommands=True)
-            cfg = self.merge_config(cfg, cfg_env)
-
-        elif defaults:
-            cfg = self.merge_config(cfg, self.get_defaults(skip_check=True))
-            with load_value_context(self.parser_mode):
-                ActionTypeHint.apply_appends(self, cfg)
+        if defaults:
             with lenient_check_context():
                 ActionTypeHint.add_sub_defaults(self, cfg)
 
@@ -317,6 +304,26 @@ def _parse_common(
         return cfg
 
 
+    def _parse_defaults_and_environ(
+        self,
+        defaults: bool = True,
+        env: Optional[bool] = None,
+        environ: Union[Dict[str, str], os._Environ] = None,
+    ):
+        cfg = Namespace()
+        if defaults:
+            cfg = self.get_defaults(skip_check=True)
+
+        if env or (env is None and self._default_env):
+            if environ is None:
+                environ = os.environ
+            with load_value_context(self.parser_mode):
+                cfg_env = self._load_env_vars(env=environ, defaults=defaults)
+            cfg = self.merge_config(cfg_env, cfg)
+
+        return cfg
+
+
     def parse_args(  # type: ignore[override]
         self,
         args: Optional[Sequence[str]] = None,
@@ -348,7 +355,11 @@ def parse_args(  # type: ignore[override]
         argcomplete_autocomplete(self)
 
         try:
-            cfg, unk = self.parse_known_args(args=args, namespace=namespace)
+            cfg = self._parse_defaults_and_environ(defaults, env)
+            if namespace:
+                cfg = self.merge_config(namespace, cfg)
+
+            cfg, unk = self.parse_known_args(args=args, namespace=cfg)
             if unk:
                 self.error(f'Unrecognized arguments: {" ".join(unk)}')
 
@@ -392,7 +403,12 @@ def parse_object(
             ParserError: If there is a parsing error and error_handler=None.
         """
         try:
-            cfg = self._apply_actions(cfg_obj)
+            cfg = self._parse_defaults_and_environ(defaults, env)
+            if cfg_base:
+                cfg = self.merge_config(cfg_base, cfg)
+
+            cfg_obj = self._apply_actions(cfg_obj)
+            cfg = self.merge_config(cfg_obj, cfg)
 
             parsed_cfg = self._parse_common(
                 cfg=cfg,
@@ -401,7 +417,6 @@ def parse_object(
                 with_meta=with_meta,
                 skip_check=_skip_check,
                 skip_required=_skip_required,
-                cfg_base=cfg_base,
                 log_message='Parsed object.',
             )
 
@@ -411,7 +426,7 @@ def parse_object(
         return parsed_cfg
 
 
-    def _load_env_vars(self, env: Dict[str, str], defaults: bool) -> Namespace:
+    def _load_env_vars(self, env: Union[Dict[str, str], os._Environ], defaults: bool) -> Namespace:
         cfg = Namespace()
         actions = filter_default_actions(self._actions)
         for action in actions:
@@ -436,10 +451,11 @@ def _load_env_vars(self, env: Dict[str, str], defaults: bool) -> Namespace:
                         try:
                             env_val = load_value(env_val)
                         except get_loader_exceptions():
-                            env_val = [env_val]  # type: ignore
+                            env_val = [env_val]
                     else:
-                        env_val = [env_val]  # type: ignore
+                        env_val = [env_val]
                 cfg[action.dest] = self._check_value_key(action, env_val, action.dest, cfg)
+        self._apply_actions(cfg)
         return cfg
 
 
@@ -465,16 +481,11 @@ def parse_env(
             ParserError: If there is a parsing error and error_handler=None.
         """
         try:
-            if env is None:
-                env = dict(os.environ)
-            with load_value_context(self.parser_mode):
-                cfg = self._load_env_vars(env=env, defaults=defaults)
-
-            self._apply_actions(cfg)
+            cfg = self._parse_defaults_and_environ(defaults, env=True, environ=env)
 
             parsed_cfg = self._parse_common(
                 cfg=cfg,
-                env=False,
+                env=True,
                 defaults=defaults,
                 with_meta=with_meta,
                 skip_check=_skip_check,
@@ -562,6 +573,10 @@ def parse_string(
             with load_value_context(self.parser_mode):
                 cfg = self._load_config_parser_mode(cfg_str, cfg_path, ext_vars, previous_config.get())
 
+            if defaults or env:
+                cfg_base = self._parse_defaults_and_environ(defaults, env)
+                cfg = self.merge_config(cfg, cfg_base)
+
             parsed_cfg = self._parse_common(
                 cfg=cfg,
                 env=env,
@@ -920,19 +935,19 @@ def get_defaults(self, skip_check: bool = False) -> Namespace:
                 cfg_file = self._load_config_parser_mode(default_config_file.get_content())
                 if key is not None:
                     cfg_file = cfg_file.get(key)
+                cfg = self.merge_config(cfg_file, cfg)
                 try:
                     with _ActionPrintConfig.skip_print_config():
-                        cfg_file = self._parse_common(
-                            cfg=cfg_file,
-                            env=None,
+                        cfg = self._parse_common(
+                            cfg=cfg,
+                            env=False,
                             defaults=False,
                             with_meta=None,
                             skip_check=skip_check,
                             skip_required=True,
                         )
                 except (TypeError, KeyError, ParserError) as ex:
                     raise ParserError(f'Problem in default config file "{default_config_file}" :: {ex.args[0]}') from ex
-            cfg = self.merge_config(cfg_file, cfg)
             meta = cfg.get('__default_config__')
             if isinstance(meta, list):
                 meta.append(default_config_file)
@@ -1217,6 +1232,7 @@ def merge_config(self, cfg_from: Namespace, cfg_to: Namespace) -> Namespace:
         cfg = cfg_to.clone()
         ActionTypeHint.discard_init_args_on_class_path_change(self, cfg, cfg_from)
         cfg.update(cfg_from)
+        ActionTypeHint.apply_appends(self, cfg)
         return cfg
 
 

jsonargparse/jsonnet.py

@@ -95,12 +95,7 @@ def __call__(self, *args, **kwargs):
 
     def _check_type(self, value, cfg):
         islist = _is_action_value_list(self)
-        ext_vars = {}
-        if self._ext_vars is not None:
-            try:
-                ext_vars = cfg[self._ext_vars]
-            except KeyError:
-                pass
+        ext_vars = cfg.get(self._ext_vars, {})
         if not islist:
             value = [value]
         for num, val in enumerate(value):

jsonargparse/typehints.py

@@ -32,7 +32,7 @@
 )
 
 from .actions import _ActionHelpClassPath, _find_action, _find_parent_action, _is_action_value_list
-from .loaders_dumpers import get_loader_exceptions, load_value
+from .loaders_dumpers import get_loader_exceptions, load_value, load_value_context
 from .namespace import Namespace
 from .typing import is_final_class, registered_types
 from .optionals import (
@@ -297,7 +297,8 @@ def apply_appends(parser, cfg):
         for key in [k for k in cfg.keys() if k.endswith('+')]:
             action = _find_action(parser, key[:-1])
             if ActionTypeHint.supports_append(action):
-                val = action._check_type(cfg[key], append=True, cfg=cfg)
+                with load_value_context(parser.parser_mode):
+                    val = action._check_type(cfg[key], append=True, cfg=cfg)
                 cfg[key[:-1]] = val
                 cfg.pop(key)
 

jsonargparse_tests/test_core.py

@@ -296,7 +296,8 @@ def test_precedence_of_sources(self):
         cfg = parser.parse_args(['--cfg', input1_config_file])
         cfg_list = parser.get_config_files(cfg)
         self.assertEqual(default_config_file, str(cfg_list[0]))
-        self.assertEqual(input1_config_file, str(cfg_list[1]))
+        self.assertEqual(input2_config_file, str(cfg_list[1]))  # From os.environ['APP_CFG']
+        self.assertEqual(input1_config_file, str(cfg_list[2]))
 
         for key in ['APP_CFG', 'APP_OP1']:
             del os.environ[key]

jsonargparse_tests/test_typehints.py

@@ -980,6 +980,35 @@ def test_default_path_unregistered_type(self):
         self.assertIn('(type: Path_drw_skip_check, default: test)', out.getvalue())
 
 
+    def test_list_append_default_config_files(self):
+        config_path = pathlib.Path(self.tmpdir, 'config.yaml')
+        parser = ArgumentParser(default_config_files=[str(config_path)])
+        parser.add_argument('--nums', type=List[int], default=[0])
+
+        with self.subTest('replace in default config'):
+            config_path.write_text('nums: [1]\n')
+            cfg = parser.parse_args(['--nums+=2'])
+            self.assertEqual(cfg.nums, [1, 2])
+            cfg = parser.parse_args(['--nums+=[2, 3]'])
+            self.assertEqual(cfg.nums, [1, 2, 3])
+
+        with self.subTest('append in default config'):
+            config_path.write_text('nums+: [1]\n')
+            cfg = parser.get_defaults()
+            self.assertEqual(cfg.nums, [0, 1])
+            cfg = parser.parse_args(['--nums+=2'])
+            self.assertEqual(cfg.nums, [0, 1, 2])
+            cfg = parser.parse_args(['--nums+=[2, 3]'])
+            self.assertEqual(cfg.nums, [0, 1, 2, 3])
+
+        with self.subTest('two default config appends'):
+            config_path2 = pathlib.Path(self.tmpdir, 'config2.yaml')
+            config_path2.write_text('nums+: [2]\n')
+            parser.default_config_files += [str(config_path2)]
+            cfg = parser.get_defaults()
+            self.assertEqual(cfg.nums, [0, 1, 2])
+
+
     def test_class_type_with_default_config_files(self):
         config = {
             'class_path': 'calendar.Calendar',