FromConfigMixin.from_config now supports subclasses (#822)

Author: mauvilsa

CHANGELOG.rst

@@ -20,6 +20,8 @@ Added
 - Signature methods now when given ``sub_configs=True``, list of paths types can
   now receive a file containing a list of paths (`#816
   <https://github.com/omni-us/jsonargparse/pull/816>`__).
+- ``FromConfigMixin.from_config`` now supports subclasses (`#822
+  <https://github.com/omni-us/jsonargparse/pull/822>`__).
 
 Fixed
 ^^^^^
@@ -52,7 +54,7 @@ Fixed
 - Union types with str and default comment-like string incorrectly parsed as a
   stringified exception of an other subtype (`#812
   <https://github.com/omni-us/jsonargparse/pull/812>`__).
-- ``FromConfig`` not handling correctly required parameters (`#813
+- ``FromConfigMixin`` not handling correctly required parameters (`#813
   <https://github.com/omni-us/jsonargparse/pull/813>`__).
 
 Changed

jsonargparse/_from_config.py

@@ -4,7 +4,12 @@
 from pathlib import Path
 from typing import Optional, Type, TypeVar, Union
 
+from ._common import parser_context
 from ._core import ArgumentParser
+from ._loaders_dumpers import get_loader_exceptions, load_value
+from ._optionals import _get_config_read_mode
+from ._typehints import is_subclass_spec, resolve_class_path_by_name
+from ._util import import_object
 
 __all__ = ["FromConfigMixin"]
 
@@ -48,23 +53,42 @@ def from_config(cls: Type[T], config: Union[str, PathLike, dict]) -> T:
         Args:
             config: Path to a config file or a dict with config values.
         """
-        kwargs = _parse_class_kwargs_from_config(cls, config, **cls.__from_config_parser_kwargs__)  # type: ignore[attr-defined]
+        kwargs, cls = _parse_class_kwargs_from_config(cls, config, **cls.__from_config_parser_kwargs__)  # type: ignore[attr-defined]
         return cls(**kwargs)
 
 
-def _parse_class_kwargs_from_config(cls: Type[T], config: Union[str, PathLike, dict], **kwargs) -> dict:
+def _parse_class_kwargs_from_config(cls: Type[T], config: Union[str, PathLike, dict], **kwargs) -> tuple[dict, Type[T]]:
     """Parse the init kwargs for ``cls`` from a config file or dict."""
     parser = ArgumentParser(exit_on_error=False, **kwargs)
+    if not isinstance(config, dict):
+        from .typing import Path
+
+        cfg_path = Path(config, mode=_get_config_read_mode())
+        cfg_str = cfg_path.get_content()
+        with parser_context(load_value_mode=parser.parser_mode):
+            try:
+                config = load_value(cfg_str, path=str(config))
+            except get_loader_exceptions() as ex:
+                raise TypeError(f"Problems parsing config '{config}': {ex}") from ex
+
+    if not isinstance(config, dict):
+        raise TypeError(f"Expected config to be a dict or parse into a dict: {config}")
+
+    if is_subclass_spec(config):
+        class_path = resolve_class_path_by_name(cls, config["class_path"])
+        obj = import_object(class_path)
+        if not issubclass(obj, cls):
+            raise TypeError(f"Class '{class_path}' is not a subclass of '{cls.__name__}'")
+        cls = obj
+        config = {**config.get("init_args", {}), **config.get("dict_kwargs", {})}
+
     parser.add_class_arguments(cls)
     for required in parser.required_args:
         action = next((a for a in parser._actions if a.dest == required), None)
         action._required = False  # type: ignore[union-attr]
     parser.required_args.clear()
-    if isinstance(config, dict):
-        cfg = parser.parse_object(config, defaults=False)
-    else:
-        cfg = parser.parse_path(config, defaults=False)
-    return parser.instantiate_classes(cfg).as_dict()
+    cfg = parser.parse_object(config, defaults=False)
+    return parser.instantiate_classes(cfg).as_dict(), cls
 
 
 def _override_init_defaults(cls: Type[T], parser_kwargs: dict) -> None:
@@ -75,7 +99,7 @@ def _override_init_defaults(cls: Type[T], parser_kwargs: dict) -> None:
     if not (isinstance(config, (str, PathLike)) and Path(config).is_file()):
         return
 
-    defaults = _parse_class_kwargs_from_config(cls, config, **parser_kwargs)
+    defaults, cls = _parse_class_kwargs_from_config(cls, config, **parser_kwargs)
     _override_init_defaults_this_class(cls, defaults)
     _override_init_defaults_parent_classes(cls, defaults)
 

jsonargparse_tests/test_from_config.py

@@ -3,7 +3,8 @@
 import pytest
 
 from jsonargparse import FromConfigMixin
-from jsonargparse_tests.conftest import json_or_yaml_dump, skip_if_omegaconf_unavailable
+from jsonargparse._paths import PathError
+from jsonargparse_tests.conftest import json_or_yaml_dump, skip_if_no_pyyaml, skip_if_omegaconf_unavailable
 
 # __init__ defaults override tests
 
@@ -178,16 +179,47 @@ class DefaultsOverrideInvalid(DefaultsOverrideParent):
 # from_config method tests
 
 
+class FromConfigMethodParent(FromConfigMixin):
+    def __init__(self, parent_param: str = "parent_default"):
+        self.parent_param = parent_param
+
+
+class FromConfigMethodChild(FromConfigMethodParent):
+    def __init__(self, child_param: str = "child_default", **kwargs):
+        super().__init__(**kwargs)
+        self.child_param = child_param
+
+
 def test_from_config_method_path(tmp_cwd):
     config_path = tmp_cwd / "config.yaml"
-    config_path.write_text(json_or_yaml_dump({"param": "value_from_file"}))
+    config_path.write_text(json_or_yaml_dump({"parent_param": "value_from_file"}))
+
+    instance = FromConfigMethodParent.from_config(config_path)
+    assert instance.parent_param == "value_from_file"
+
+
+def test_from_config_method_path_does_not_exist(tmp_cwd):
+    config_path = tmp_cwd / "config.yaml"
+
+    with pytest.raises(PathError, match="File does not exist:"):
+        FromConfigMethodParent.from_config(config_path)
+
+
+@skip_if_no_pyyaml
+def test_from_config_method_path_invalid_yaml(tmp_cwd):
+    config_path = tmp_cwd / "config.yaml"
+    config_path.write_text("::: invalid content :::")
+
+    with pytest.raises(TypeError, match="Problems parsing config"):
+        FromConfigMethodParent.from_config(config_path)
 
-    class FromConfigMethodPath(FromConfigMixin):
-        def __init__(self, param: str = "default_value"):
-            self.param = param
 
-    instance = FromConfigMethodPath.from_config(config_path)
-    assert instance.param == "value_from_file"
+def test_from_config_method_path_not_dict(tmp_cwd):
+    config_path = tmp_cwd / "config.yaml"
+    config_path.write_text("[1, 2]")
+
+    with pytest.raises(TypeError, match="Expected config to be a dict or parse into a dict"):
+        FromConfigMethodParent.from_config(config_path)
 
 
 def test_from_config_method_dict():
@@ -224,15 +256,6 @@ def __init__(self, param1: str = "default_value", param2: int = 1):
 
 
 def test_from_config_method_subclass():
-    class FromConfigMethodParent(FromConfigMixin):
-        def __init__(self, parent_param: str = "parent_default"):
-            self.parent_param = parent_param
-
-    class FromConfigMethodChild(FromConfigMethodParent):
-        def __init__(self, child_param: str = "child_default", **kwargs):
-            super().__init__(**kwargs)
-            self.child_param = child_param
-
     instance = FromConfigMethodChild.from_config(
         {"parent_param": "overridden_parent", "child_param": "overridden_child"}
     )
@@ -241,6 +264,27 @@ def __init__(self, child_param: str = "child_default", **kwargs):
     assert instance.child_param == "overridden_child"
 
 
+def test_from_config_method_class_path_subclass():
+    instance = FromConfigMethodParent.from_config(
+        {
+            "class_path": f"{__name__}.FromConfigMethodChild",
+            "init_args": {"parent_param": "overridden_parent", "child_param": "overridden_child"},
+        }
+    )
+    assert isinstance(instance, FromConfigMethodChild)
+    assert instance.parent_param == "overridden_parent"
+    assert instance.child_param == "overridden_child"
+
+
+class SomeOtherClass:
+    pass
+
+
+def test_from_config_method_class_path_not_subclass():
+    with pytest.raises(TypeError, match="SomeOtherClass' is not a subclass of 'FromConfigMethodParent'"):
+        FromConfigMethodParent.from_config({"class_path": f"{__name__}.SomeOtherClass"})
+
+
 @skip_if_omegaconf_unavailable
 def test_from_config_method_parser_kwargs():
     class FromConfigMethodParserKwargs(FromConfigMixin):