Always normalize defaults

CHANGELOG.rst

@@ -73,6 +73,9 @@ Fixed
   <https://github.com/mauvilsa/jsonargparse/pull/982>`__).
 - Instantiate links failing when the value set on a subclass target is a class
   path dict (`#982 <https://github.com/mauvilsa/jsonargparse/pull/982>`__).
+- After a ``parse_args`` call, ``parse_object`` and ``parse_string`` with
+  ``defaults=False`` incorrectly adding the defaults of subclass ``init_args``
+  (`#983 <https://github.com/mauvilsa/jsonargparse/pull/983>`__).
 
 Changed
 ^^^^^^^
@@ -126,6 +129,12 @@ Changed
   are meant to be given in ``init_args``, and the ``jsonschema`` completion type
   only describes them there, see :ref:`unresolved-parameters` (`#981
   <https://github.com/mauvilsa/jsonargparse/pull/981>`__).
+- Defaults are now always normalized. Invalid defaults still only fail when
+  ``validate_defaults`` is enabled (`#983
+  <https://github.com/mauvilsa/jsonargparse/pull/983>`__).
+- A ``types.ModuleType`` value given as a module object is now normalized to its
+  import path, instead of only being normalized when given as a default (`#983
+  <https://github.com/mauvilsa/jsonargparse/pull/983>`__).
 
 Removed
 ^^^^^^^

DOCUMENTATION.rst

@@ -564,8 +564,9 @@ Types can be nested with any complexity. Notes about the support:
   :ref:`stubs-resolver`), that is the callable's return type or a subclass of
   it. Argument types are not validated.
 
-- ``types.ModuleType`` accepts the dot import path of a module, and on
-  ``instantiate`` is replaced by the imported module object.
+- ``types.ModuleType`` accepts the dot import path of a module, or a module
+  object which is normalized to its import path, and on ``instantiate`` is
+  replaced by the imported module object.
 
 - ``types.UnionType`` and ``types.GenericAlias``, commonly found in third party
   libraries in unions such as ``type | UnionType | dict``, accept a string with
@@ -1122,7 +1123,7 @@ Parsing complex-valued points:
 
     >>> parser.add_argument("--point", type=Point2d[complex])  # doctest: +IGNORE_RESULT
     >>> parser.parse_args(["--point.x=(1+2j)"]).point
-    Namespace(x=(1+2j), y=0.0)
+    Namespace(x=(1+2j), y=0j)
 
 A ``TypeVar`` can't be used to validate, so when it is used as a type, e.g.
 ``options: Optional[OptionsT] = None``, it is replaced by what it stands for:

jsonargparse/_common.py

@@ -377,8 +377,9 @@ def set_parsing_settings(
 
     Args:
         validate_defaults: Whether default values must be valid according to the
-            argument type. The default is ``False``, meaning no default
-            validation, like in argparse.
+            argument type. Defaults are always validated to normalize them, but
+            with the default ``False`` an invalid default is kept as is, like
+            in argparse.
         validate_subclass_spec_in_any: If ``True``, when a value for a type that
             accepts any value, i.e. ``Any``, ``object``, ``Unvalidated<...>`` or
             a dict that doesn't validate its values, looks like a subclass spec
@@ -497,24 +498,27 @@ def get_parsing_setting(name: str):
     return parsing_settings[name]
 
 
-def validate_default(container: ActionsContainer, action: argparse.Action):
-    if (
-        action.default is get_parsing_setting("unset_sentinel")
-        or not get_parsing_setting("validate_defaults")
-        or not hasattr(action, "_check_type")
-    ):
+def validate_default(container: ActionsContainer, action: argparse.Action, logger: logging.Logger) -> None:
+    if action.default is get_parsing_setting("unset_sentinel") or not hasattr(action, "_check_type"):
         return
+    from ._core import ArgumentGroup
+    from ._typehints import ActionTypeHint, normalize_default_value
+
+    default = action.default
+    if isinstance(action, ActionTypeHint):
+        default = normalize_default_value(action, default)
+    if isinstance(container, ArgumentGroup):
+        container = container.parser
     try:
-        from ._core import ArgumentGroup
-
-        if isinstance(container, ArgumentGroup):
-            container = container.parser
         with parser_context(parent_parser=container, validating_defaults=True):
-            default = action.default
             action.default = None
             action.default = action._check_type_(default)  # type: ignore[attr-defined]
     except Exception as ex:
-        raise ValueError(f"Default value is not valid: {ex}") from ex
+        action.default = default
+        msg = f"Default value is not valid: {ex}"
+        if get_parsing_setting("validate_defaults"):
+            raise ValueError(msg) from ex
+        logger.debug(msg)
 
 
 def get_optionals_as_positionals_actions(parser, include_positionals=False):

jsonargparse/_core.py

@@ -196,7 +196,7 @@ def add_argument(self, *args, sub_configs: bool = False, **kwargs):
         unset_sentinel = get_parsing_setting("unset_sentinel")
         if unset_sentinel is not None and "default" not in kwargs and action.default is None:
             action.default = unset_sentinel
-        validate_default(self, action)
+        validate_default(self, action, self._logger)
         return action
 
     def add_argument_group(self, *args, name: str | None = None, **kwargs) -> "ArgumentGroup":
@@ -248,9 +248,9 @@ def set_defaults(self, *args: dict[str, Any], **kwargs: Any) -> None:
                     default = {f"{dest}.{k}": v for k, v in default.items()}
                     self.set_defaults(default)
                     continue
-                if isinstance(action, ActionTypeHint):
-                    default = action.normalize_default(default)
                 self._defaults[dest] = action.default = default
+                validate_default(self, action, self._logger)
+                self._defaults[dest] = action.default
         if kwargs:
             self.set_defaults(kwargs)
 

jsonargparse/_formatters.py

@@ -11,6 +11,7 @@
     _SubParsersAction,
 )
 from collections.abc import Iterable
+from enum import Enum
 from io import StringIO
 from string import Template
 
@@ -38,9 +39,11 @@
     ActionTypeHint,
     get_optional_arg,
     get_subclass_or_closed_types,
+    is_callable_type,
     is_subclass_spec,
     type_to_str,
 )
+from ._util import object_path_serializer
 
 __all__ = ["DefaultHelpFormatter"]
 
@@ -535,6 +538,18 @@ def _expand_help(self, action: Action) -> str:
                 params["default"] = "null"
             elif isinstance(params["default"], Namespace):
                 params["default"] = params["default"].as_dict()
+            elif isinstance(params["default"], Enum):
+                params["default"] = params["default"].name
+            elif (
+                isinstance(action, ActionTypeHint)
+                and is_callable_type(action._typehint)
+                and callable(params["default"])
+            ):
+                try:
+                    params["default"] = object_path_serializer(params["default"])
+                except ValueError:
+                    # kept as is when it can't be imported back, e.g. a closure
+                    pass
         help_str = PercentTemplate(self._get_help_string(action)).safe_substitute(params)
         action.default = orig_default
         return help_str

jsonargparse/_subcommands.py

@@ -169,8 +169,11 @@ def __call__(self, parser, namespace, values, option_string=None):
 
 @contextmanager
 def parse_kwargs_context(kwargs):
-    parse_kwargs.set(kwargs)
-    yield
+    token = parse_kwargs.set(kwargs)
+    try:
+        yield
+    finally:
+        parse_kwargs.reset(token)
 
 
 def get_subcommands(

jsonargparse/_typehints.py

@@ -408,42 +408,6 @@ def __init__(self, typehint: type | None = None, enable_path: bool = False, **kw
                 kwargs["metavar"] = typehint_metavar(self._typehint)
             super().__init__(**kwargs)
             self._supports_append = self.supports_append(self._typehint)
-            self.default = self.normalize_default(self.default)
-
-    def normalize_default(self, default):
-        from ._signatures import convert_to_dict, is_convertible_to_dict
-
-        is_subclass_type = self.is_subclass_typehint(self._typehint, all_subtypes=False)
-        if isinstance(default, _LazyInitBaseClass):
-            default = default.lazy_get_init_data().as_dict()
-        elif is_convertible_to_dict(default.__class__):
-            default = convert_to_dict(default)
-        elif is_subclass_type and isinstance(default, dict) and "class_path" in default:
-            default = subclass_spec_as_namespace(default)
-            default.class_path = normalize_import_path(default.class_path, self._typehint)
-        elif is_enum_type(self._typehint) and isinstance(default, Enum):
-            default = default.name
-        elif is_module_type(self._typehint) and isinstance(default, ModuleType):
-            default = default.__name__
-        elif is_callable_type(self._typehint) and callable(default) and not inspect.isclass(default):
-            try:
-                default = object_path_serializer(default)
-            except ValueError:
-                # kept as is when it can't be imported back, e.g. a closure, so that dump warns
-                pass
-        elif ActionTypeHint.is_return_subclass_typehint(self._typehint) and inspect.isclass(default):
-            default = {"class_path": get_import_path(default)}
-        elif is_subclass_type and not allow_default_instance.get():
-            from ._parameter_resolvers import UnknownDefault
-
-            default_type = type(default)
-            if (
-                not is_subclass(default_type, UnknownDefault)
-                and self.is_subclass_typehint(default_type)
-                and not any(implements_protocol(default, t) for t in get_subclass_types(self._typehint) or ())
-            ):
-                raise ValueError("Subclass types require as default either a dict with class_path or a lazy instance.")
-        return default
 
     @staticmethod
     def prepare_add_argument(args, kwargs, enable_path, container, logger, sub_add_kwargs=None):
@@ -875,6 +839,32 @@ def completer(self, prefix, **kwargs):
             return argcomplete_warn_redraw_prompt(prefix, msg)
 
 
+def normalize_default_value(action, default):
+    from ._signatures import convert_to_dict, is_convertible_to_dict
+
+    is_subclass_type = action.is_subclass_typehint(action._typehint, all_subtypes=False)
+    if isinstance(default, _LazyInitBaseClass):
+        default = default.lazy_get_init_data().as_dict()
+    elif is_convertible_to_dict(default.__class__):
+        default = convert_to_dict(default)
+    elif is_subclass_type and isinstance(default, dict) and "class_path" in default:
+        default = subclass_spec_as_namespace(default)
+        default.class_path = normalize_import_path(default.class_path, action._typehint)
+    elif ActionTypeHint.is_return_subclass_typehint(action._typehint) and inspect.isclass(default):
+        default = {"class_path": get_import_path(default)}
+    elif is_subclass_type and not allow_default_instance.get():
+        from ._parameter_resolvers import UnknownDefault
+
+        default_type = type(default)
+        if (
+            not is_subclass(default_type, UnknownDefault)
+            and action.is_subclass_typehint(default_type)
+            and not any(implements_protocol(default, t) for t in get_subclass_types(action._typehint) or ())
+        ):
+            raise ValueError("Subclass types require as default either a dict with class_path or a lazy instance.")
+    return default
+
+
 def is_pathlike(typehint) -> bool:
     if get_typehint_origin(typehint) == Union:
         return any(is_pathlike(t) for t in typehint.__args__)
@@ -1490,7 +1480,7 @@ def adapt_typehints(
     # Module
     elif typehint is ModuleType:
         if isinstance(val, ModuleType):
-            if serialize:
+            if not instantiate_classes:
                 val = val.__name__
         else:
             if isinstance(val, str):
@@ -2892,12 +2882,6 @@ def get_optional_arg(annotation, ref_type=None):
     return annotation
 
 
-def is_enum_type(annotation):
-    return is_subclass(annotation, Enum) or (
-        get_typehint_origin(annotation) == Union and any(is_subclass(a, Enum) for a in annotation.__args__)
-    )
-
-
 def is_module_type(annotation):
     annotation = get_unaliased_type(annotation)
     return annotation is ModuleType or (

jsonargparse_tests/test_link_arguments.py

@@ -364,13 +364,13 @@ def get_linked_default_parser(**init_args):
 def test_on_parse_linked_init_arg_removed_from_default(subtests):
     with subtests.test("other init args kept"):
         parser = get_linked_default_parser(p1=2, p2="x")
-        expected = {"class_path": f"{__name__}.LinkedDefaultTarget", "init_args": {"p2": "x"}}
+        expected = Namespace(class_path=f"{__name__}.LinkedDefaultTarget", init_args=Namespace(p2="x"))
         assert parser.get_default("b") == expected
         assert parser.parse_args([]).b.init_args == Namespace(p1=1, p2="x")
 
     with subtests.test("init args removed"):
         parser = get_linked_default_parser(p1=2)
-        assert parser.get_default("b") == {"class_path": f"{__name__}.LinkedDefaultTarget"}
+        assert parser.get_default("b") == Namespace(class_path=f"{__name__}.LinkedDefaultTarget")
 
 
 class ClassF:

jsonargparse_tests/test_paths.py

@@ -1195,7 +1195,7 @@ def test_sub_configs_list_path_fr(parser, tmp_cwd, mock_stdin, subtests):
     with subtests.test("paths list nargs='+' path not exist"):
         pytest.raises(ArgumentError, lambda: parser.parse_args(["--lists", str(list_file4)]))
 
-    with subtests.test("paths list nargs='+' list not exist"):  # TODO: check error message
+    with subtests.test("paths list nargs='+' list not exist"):
         with pytest.raises(ArgumentError) as ctx:
             parser.parse_args(["--lists", "no-such-file"])
         ctx.match("Expected a path but no-such-file either not accessible or invalid")

jsonargparse_tests/test_subclasses.py

@@ -242,6 +242,13 @@ def test_subclass_parse_defaults_disabled(parser):
     assert cfg.op == Namespace(class_path=f"{__name__}.DefaultsDisabled", init_args=Namespace(p1=3))
 
 
+def test_subclass_parse_object_defaults_disabled_after_parse_args(parser):
+    parser.add_argument("--op", type=DefaultsDisabled)
+    parser.parse_args([])
+    cfg = parser.parse_object({"op": {"init_args": {"p1": 3}}}, defaults=False)
+    assert cfg.op == Namespace(class_path=f"{__name__}.DefaultsDisabled", init_args=Namespace(p1=3))
+
+
 def test_subclass_known_subclasses(parser):
     parser.add_argument("--obj", type=BaseC)
     help_str = get_parser_help(parser)
@@ -1278,7 +1285,9 @@ def __init__(self, obj: Union[BaseC, bool] = lazy_instance(OverrideMixed, param=
 def test_subclass_discard_init_args_mixed_type(parser, logger):
     _cached_class_parsers.clear()
     parser.logger = logger
-    parser.add_class_arguments(OverrideMixedMain, "main")
+    with capture_logs(logger) as logs:
+        parser.add_class_arguments(OverrideMixedMain, "main")
+    assert "Parsed object: Namespace(param=1)" in logs.getvalue()
     with capture_logs(logger) as logs:
         parser.parse_args(["--main.obj=BaseC"])
     assert "discarding init_args: {'param': 1}" in logs.getvalue()
@@ -1302,7 +1311,9 @@ def __init__(self, s2: str = "-"):
 def test_subclass_discard_init_args_config_with_default(parser, logger):
     parser.logger = logger
     parser.add_argument("--cfg", action="config")
-    parser.add_argument("--s", type=OverrideBase, default=lazy_instance(OverrideSub1, s1="v1"))
+    with capture_logs(logger) as logs:
+        parser.add_argument("--s", type=OverrideBase, default=lazy_instance(OverrideSub1, s1="v1"))
+    assert "Parsed object: Namespace(s1='v1')" in logs.getvalue()
 
     config = {"s": {"class_path": "OverrideSub2", "init_args": {"s2": "v2"}}}
     with capture_logs(logger) as logs:
@@ -1338,9 +1349,7 @@ def test_subclass_discard_init_args_with_default_config_files(parser, tmp_cwd, l
         cfg = parser.parse_args([f'--obj={{"class_path": "{__name__}.BaseC", "init_args": {{"p": 3}}}}'])
     assert "discarding init_args: {'param': '1'}" in logs.getvalue()
     assert cfg.obj.init_args == Namespace(p=3)
-    with capture_logs(logger) as logs:
-        assert type(parser.instantiate(cfg).obj) is BaseC
-    assert logs.getvalue()
+    assert type(parser.instantiate(cfg).obj) is BaseC
 
 
 class Arch:
@@ -1408,7 +1417,9 @@ def test_discard_init_args_config_nested(parser, logger, tmp_cwd, method):
     }
     if method == "class":
         config = {"main": subconfig}
-        parser.add_class_arguments(ConfigDiscardMain, "main")
+        with capture_logs(logger) as logs:
+            parser.add_class_arguments(ConfigDiscardMain, "main")
+        assert "Parsed object: Namespace()" in logs.getvalue()
     else:
         config = {
             "main": {
@@ -1417,17 +1428,17 @@ def test_discard_init_args_config_nested(parser, logger, tmp_cwd, method):
             }
         }
         parser.add_subclass_arguments(ConfigDiscardMain, "main")
-        parser.set_defaults(main=lazy_instance(ConfigDiscardMain))
+        with capture_logs(logger) as logs:
+            parser.set_defaults(main=lazy_instance(ConfigDiscardMain))
+        assert "Parsed object: Namespace()" in logs.getvalue()
 
     config_path = Path("config.yaml")
     config_path.write_text(json_or_yaml_dump(config))
 
     with capture_logs(logger) as logs:
         cfg = parser.parse_args([f"--cfg={config_path}"])
     assert "discarding init_args: {'s1': 'x'}" in logs.getvalue()
-    with capture_logs(logger) as logs:
-        init = parser.instantiate(cfg)
-    assert logs.getvalue()
+    init = parser.instantiate(cfg)
     assert isinstance(init.main, ConfigDiscardMain)
     assert isinstance(init.main.sub, ConfigDiscardSub2)
 
@@ -1456,7 +1467,9 @@ def test_subclass_discard_init_args_dict_looks_like_subclass(parser, logger, tmp
     parser.logger = logger
     parser.add_argument("--cfg", action="config")
     parser.add_subclass_arguments(DictDiscardMain, "main")
-    parser.set_defaults(main=lazy_instance(DictDiscardMain))
+    with capture_logs(logger) as logs:
+        parser.set_defaults(main=lazy_instance(DictDiscardMain))
+    assert "Parsed object: Namespace()" in logs.getvalue()
 
     configs, subconfigs, config_paths = {}, {}, {}
     for c in [1, 2]:
@@ -1478,9 +1491,7 @@ def test_subclass_discard_init_args_dict_looks_like_subclass(parser, logger, tmp
     with capture_logs(logger) as logs:
         cfg = parser.parse_args([f"--cfg={config_paths[1]}", f"--cfg={config_paths[2]}"])
     assert "discarding init_args: {'s1': 1}" in logs.getvalue()
-    with capture_logs(logger) as logs:
-        init = parser.instantiate(cfg)
-    assert logs.getvalue()
+    init = parser.instantiate(cfg)
     assert isinstance(init.main, DictDiscardMain)
     assert isinstance(init.main.sub, dict)
     assert init.main.sub["init_args"]["s2"] == 2