Fix issue with import of subgroups from helpers

Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com>

Author: lebrice

simple_parsing/helpers/__init__.py

@@ -14,6 +14,7 @@
 from .hparams import HyperParameters
 from .partial import Partial, config_for
 from .serialization import FrozenSerializable, Serializable, SimpleJsonEncoder, encode
+from .subgroups import subgroups
 
 try:
     from .serialization import YamlSerializable
@@ -45,4 +46,5 @@
     "subparsers",
     "flag",
     "flags",
+    "subgroups",
 ]

simple_parsing/parsing.py

@@ -1,4 +1,5 @@
 """Simple, Elegant Argument parsing.
+
 @author: Fabrice Normandin
 """
 from __future__ import annotations
@@ -98,7 +99,6 @@ class ArgumentParser(argparse.ArgumentParser):
 
     - add_config_path_arg : bool, optional
         When set to `True`, adds a `--config_path` argument, of type Path, which is used to parse
-
     """
 
     def __init__(
@@ -531,8 +531,8 @@ def _preprocessing(self, args: Sequence[str] = (), namespace: Namespace | None =
         # Create one argument group per dataclass
         for wrapped_dataclass in wrapped_dataclasses:
             logger.debug(
-                f"Parser {id(self)} is Adding arguments for dataclass: {wrapped_dataclass.dataclass} "
-                f"at destinations {wrapped_dataclass.destinations}"
+                f"Parser {id(self)} is Adding arguments for dataclass: "
+                f"{wrapped_dataclass.dataclass} at destinations {wrapped_dataclass.destinations}"
             )
             wrapped_dataclass.add_arguments(parser=self)
 
@@ -636,7 +636,8 @@ def _resolve_subgroups(
             # Do rounds of parsing with just the subgroup arguments, until all the subgroups
             # are resolved to a dataclass type.
             logger.debug(
-                f"Starting subgroup parsing round {current_nesting_level}: {list(unresolved_subgroups.keys())}"
+                f"Starting subgroup parsing round {current_nesting_level}: "
+                f"{list(unresolved_subgroups.keys())}"
             )
             # Add all the unresolved subgroups arguments.
             for dest, subgroup_field in unresolved_subgroups.items():
@@ -761,6 +762,7 @@ def _resolve_subgroups(
 
     def _remove_subgroups_from_namespace(self, parsed_args: argparse.Namespace) -> None:
         """Removes the subgroup choice results from the namespace.
+
         Modifies the namespace in-place.
         """
         # find all subgroup fields
@@ -876,8 +878,9 @@ def _instantiate_dataclasses(
                     existing = getattr(parsed_args, destination)
                     if dc_wrapper.dest in self._defaults:
                         logger.debug(
-                            f"Overwriting defaults in the namespace at destination '{destination}' "
-                            f"on the Namespace ({existing}) to a value of {value_for_dataclass_field}"
+                            f"Overwriting defaults in the namespace at destination "
+                            f"'{destination}' on the Namespace ({existing}) to a value of "
+                            f"{value_for_dataclass_field}"
                         )
                         setattr(parsed_args, destination, value_for_dataclass_field)
                     else:
@@ -937,19 +940,37 @@ def _fill_constructor_arguments_with_fields(
         parsed_arg_values = vars(parsed_args)
         deleted_values: dict[str, Any] = {}
 
-        for wrapper in wrappers:
-            for field in wrapper.fields:
-                if argparse.SUPPRESS in wrapper.defaults and field.dest not in parsed_args:
+        # BUG: Need to check that the non-init fields DO have a FieldWrapper here, and that there
+        # isn't a value for that field in the constructor arguments!
+
+        for dc_wrapper in wrappers:
+            for non_init_field in [
+                f for f in dataclasses.fields(dc_wrapper.dataclass) if not f.init
+            ]:
+                field_dest = dc_wrapper.dest + "." + non_init_field.name
+                # We fetch the constructor arguments for the containing dataclass and check that it
+                # doesn't have a value set.
+                dc_constructor_args = constructor_arguments
+                for dest_part in dc_wrapper.dest.split("."):
+                    dc_constructor_args = dc_constructor_args[dest_part]
+                if non_init_field.name in dc_constructor_args:
+                    logger.warning(
+                        f"Field {field_dest} is a field with init=False, but a value is "
+                        f"present in the serialized config. This value will be ignored."
+                    )
+                    dc_constructor_args.pop(non_init_field.name)
+
+            for field in dc_wrapper.fields:
+                if argparse.SUPPRESS in dc_wrapper.defaults and field.dest not in parsed_args:
                     continue
 
                 if field.is_subgroup:
                     # Skip the subgroup fields, since we added a child DataclassWrapper for them.
                     logger.debug(f"Not calling the subgroup FieldWrapper for dest {field.dest}")
                     continue
 
-                if not field.field.init:
-                    # The field isn't an argument of the dataclass constructor.
-                    continue
+                # We only create FieldWrappers for fields that have init=True.
+                assert field.field.init
 
                 # NOTE: If the field is reused (when using the ConflictResolution.ALWAYS_MERGE
                 # strategy), then we store the multiple values in the `dest` of the first field.
@@ -979,11 +1000,12 @@ def _fill_constructor_arguments_with_fields(
     @property
     def confilct_resolver_max_attempts(self) -> int:
         return self._conflict_resolver.max_attempts
-    
+
     @confilct_resolver_max_attempts.setter
     def confilct_resolver_max_attempts(self, value: int):
         self._conflict_resolver.max_attempts = value
 
+
 # TODO: Change the order of arguments to put `args` as the second argument.
 def parse(
     config_class: type[DataclassT],
@@ -1068,7 +1090,9 @@ def parse_known_args(
         add_config_path_arg=add_config_path_arg,
     )
     parser.add_arguments(config_class, dest=dest, default=default)
-    parsed_args, unknown_args = parser.parse_known_args(args, attempt_to_reorder=attempt_to_reorder)
+    parsed_args, unknown_args = parser.parse_known_args(
+        args, attempt_to_reorder=attempt_to_reorder
+    )
     config: Dataclass = getattr(parsed_args, dest)
     return config, unknown_args
 

simple_parsing/utils.py

@@ -108,9 +108,8 @@ def is_subparser_field(field: Field) -> bool:
 
 
 class InconsistentArgumentError(RuntimeError):
-    """
-    Error raised when the number of arguments provided is inconsistent when parsing multiple instances from command line.
-    """
+    """Error raised when the number of arguments provided is inconsistent when parsing multiple
+    instances from command line."""
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
@@ -126,9 +125,8 @@ def camel_case(name):
 
 
 def str2bool(raw_value: str | bool) -> bool:
-    """
-    Taken from https://stackoverflow.com/questions/15008758/parsing-boolean-values-with-argparse
-    """
+    """Taken from https://stackoverflow.com/questions/15008758/parsing-boolean-values-with-
+    argparse."""
     if isinstance(raw_value, bool):
         return raw_value
     v = raw_value.strip().lower()
@@ -208,12 +206,13 @@ def get_item_type(container_type: type[Container[T]]) -> T:
 def get_argparse_type_for_container(
     container_type: type[Container[T]],
 ) -> type[T] | Callable[[str], T]:
-    """Gets the argparse 'type' option to be used for a given container type.
-    When an annotation is present, the 'type' option of argparse is set to that type.
-    if not, then the default value of 'str' is returned.
+    """Gets the argparse 'type' option to be used for a given container type. When an annotation is
+    present, the 'type' option of argparse is set to that type. if not, then the default value of
+    'str' is returned.
 
     Arguments:
-        container_type {Type} -- A container type (ideally a typing.Type such as List, Tuple, along with an item annotation: List[str], Tuple[int, int], etc.)
+        container_type -- A container type (ideally a typing.Type such as List, Tuple, along
+            with an item annotation: List[str], Tuple[int, int], etc.)
 
     Returns:
         typing.Type -- the type that should be used in argparse 'type' argument option.
@@ -413,7 +412,9 @@ def is_dataclass_type_or_typevar(t: type) -> bool:
     Returns:
         bool: Whether its a dataclass type.
     """
-    return dataclasses.is_dataclass(t) or (is_typevar(t) and dataclasses.is_dataclass(get_bound(t)))
+    return dataclasses.is_dataclass(t) or (
+        is_typevar(t) and dataclasses.is_dataclass(get_bound(t))
+    )
 
 
 def is_enum(t: type) -> bool:
@@ -431,7 +432,7 @@ def is_tuple_or_list(t: type) -> bool:
 
 
 def is_union(t: type) -> bool:
-    """Returns whether or not the given Type annotation is a variant (or subclass) of typing.Union
+    """Returns whether or not the given Type annotation is a variant (or subclass) of typing.Union.
 
     Args:
         t (Type): some type annotation
@@ -453,8 +454,7 @@ def is_union(t: type) -> bool:
 
 
 def is_homogeneous_tuple_type(t: type[tuple]) -> bool:
-    """Returns whether the given Tuple type is homogeneous: if all items types are the
-    same.
+    """Returns whether the given Tuple type is homogeneous: if all items types are the same.
 
     This also includes Tuple[<some_type>, ...]
 
@@ -651,19 +651,22 @@ def _parse(value: str) -> list[Any]:
             # if it doesn't work, fall back to the parse_fn.
             values = _fallback_parse(value)
 
-        # we do the default 'argparse' action, which is to add the values to a bigger list of values.
+        # we do the default 'argparse' action, which is to add the values to a bigger list of
+        # values.
         # result.extend(values)
         logger.debug(f"returning values: {values}")
         return values
 
     def _parse_literal(value: str) -> list[Any] | Any:
         """try to parse the string to a python expression directly.
+
         (useful for nested lists or tuples.)
         """
         literal = ast.literal_eval(value)
         logger.debug(f"Parsed literal: {literal}")
         if not isinstance(literal, (list, tuple)):
-            # we were passed a single-element container, like "--some_list 1", which should give [1].
+            # we were passed a single-element container, like "--some_list 1", which should give
+            # [1].
             # We therefore return the literal itself, and argparse will append it.
             return T(literal)
         else:
@@ -723,8 +726,8 @@ def get_nesting_level(possibly_nested_list):
 
 
 def default_value(field: dataclasses.Field) -> T | _MISSING_TYPE:
-    """Returns the default value of a field in a dataclass, if available.
-    When not available, returns `dataclasses.MISSING`.
+    """Returns the default value of a field in a dataclass, if available. When not available,
+    returns `dataclasses.MISSING`.
 
     Args:
         field (dataclasses.Field): The dataclasses.Field to get the default value of.
@@ -781,7 +784,6 @@ def keep_keys(d: dict, keys_to_keep: Iterable[str]) -> tuple[dict, dict]:
     Tuple[Dict, Dict]
         The same dictionary (with all the unwanted keys removed) as well as a
         new dict containing only the removed item.
-
     """
     d_keys = set(d.keys())  # save a copy since we will modify the dict.
     removed = {}
@@ -792,7 +794,7 @@ def keep_keys(d: dict, keys_to_keep: Iterable[str]) -> tuple[dict, dict]:
 
 
 def compute_identity(size: int = 16, **sample) -> str:
-    """Compute a unique hash out of a dictionary
+    """Compute a unique hash out of a dictionary.
 
     Parameters
     ----------
@@ -801,7 +803,6 @@ def compute_identity(size: int = 16, **sample) -> str:
 
     **sample:
         Dictionary to compute the hash from
-
     """
     sample_hash = hashlib.sha256()
 
@@ -840,7 +841,7 @@ def zip_dicts(*dicts: dict[K, V]) -> Iterable[tuple[K, tuple[V | None, ...]]]:
 
 
 def dict_union(*dicts: dict[K, V], recurse: bool = True, dict_factory=dict) -> dict[K, V]:
-    """Simple dict union until we use python 3.9
+    """Simple dict union until we use python 3.9.
 
     If `recurse` is True, also does the union of nested dictionaries.
     NOTE: The returned dictionary has keys sorted alphabetically.
@@ -924,7 +925,8 @@ def unflatten(flattened: Mapping[tuple[K, ...], V]) -> PossiblyNestedDict[K, V]:
 
 
 def flatten_join(nested: PossiblyNestedMapping[str, V], sep: str = ".") -> dict[str, V]:
-    """Flatten a dictionary of dictionaries. Joins different nesting levels with `sep` as separator.
+    """Flatten a dictionary of dictionaries. Joins different nesting levels with `sep` as
+    separator.
 
     >>> flatten_join({'a': {'b': 2, 'c': 3}, 'c': {'d': 3, 'e': 4}})
     {'a.b': 2, 'a.c': 3, 'c.d': 3, 'c.e': 4}

test/test_examples.py

@@ -1,6 +1,6 @@
 """A test to make sure that all the example files work without crashing.
-(Could be seen as a kind of integration test.)
 
+(Could be seen as a kind of integration test.)
 """
 from __future__ import annotations
 
@@ -134,7 +134,9 @@ def test_running_example_outputs_expected_without_arg(
     set_prog_name: Callable[[str, str | None], None],
     assert_equals_stdout: Callable[[str, str], None],
 ):
-    return test_running_example_outputs_expected(file_path, "", set_prog_name, assert_equals_stdout)
+    return test_running_example_outputs_expected(
+        file_path, "", set_prog_name, assert_equals_stdout
+    )
 
 
 @contextmanager