Add fix for default args vs predefined args

Author: timkpaine

hatch_build/cli.py

@@ -44,16 +44,18 @@ def parse_extra_args(subparser: Optional[ArgumentParser] = None) -> List[str]:
 
 
 def _is_supported_type(field_type: type) -> bool:
-    if not isinstance(field_type, type):
-        return False
     if get_origin(field_type) is Optional:
         field_type = get_args(field_type)[0]
     elif get_origin(field_type) is Union:
         non_none_types = [t for t in get_args(field_type) if t is not type(None)]
+        if all(_is_supported_type(t) for t in non_none_types):
+            return True
         if len(non_none_types) == 1:
             field_type = non_none_types[0]
     elif get_origin(field_type) is Literal:
         return all(isinstance(arg, (str, int, float, bool, Enum)) for arg in get_args(field_type))
+    if not isinstance(field_type, type):
+        return False
     return field_type in (str, int, float, bool) or issubclass(field_type, Enum)
 
 
@@ -100,6 +102,8 @@ def _recurse_add_fields(parser: ArgumentParser, model: Union["BaseModel", Type["
         # Default value, promote PydanticUndefined to None
         if field.default is PydanticUndefined:
             default_value = None
+        elif field_instance:
+            default_value = field_instance
         else:
             default_value = field.default
 
@@ -167,15 +171,36 @@ def _recurse_add_fields(parser: ArgumentParser, model: Union["BaseModel", Type["
             if get_args(field_type) and not _is_supported_type(get_args(field_type)[0]):
                 # If theres already something here, we can procede by adding the command with a positional indicator
                 if field_instance:
-                    ########################
-                    # MARK: List[BaseModel]
                     for i, value in enumerate(field_instance):
-                        _recurse_add_fields(parser, value, prefix=f"{field_name}.{i}.")
+                        if isinstance(value, BaseModel):
+                            ########################
+                            # MARK: List[BaseModel]
+                            _recurse_add_fields(parser, value, prefix=f"{field_name}.{i}.")
+                            continue
+                        else:
+                            ########################
+                            # MARK: List[str|int|float|bool]
+                            _add_argument(
+                                parser=parser,
+                                name=f"{arg_name}.{i}",
+                                arg_type=type(value),
+                                default_value=value,
+                            )
                     continue
                 # If there's nothing here, we don't know how to address them
                 # TODO: we could just prefill e.g. --field.0, --field.1 up to some limit
                 _log.warning(f"Only lists of str, int, float, or bool are supported - field `{field_name}` got {get_args(field_type)[0]}")
                 continue
+            if field_instance:
+                for i, value in enumerate(field_instance):
+                    ########################
+                    # MARK: List[str|int|float|bool]
+                    _add_argument(
+                        parser=parser,
+                        name=f"{arg_name}.{i}",
+                        arg_type=type(value),
+                        default_value=value,
+                    )
             #################################
             # MARK: List[str|int|float|bool]
             _add_argument(
@@ -414,6 +439,21 @@ def parse_extra_args_model(model: "BaseModel"):
 
                 _log.debug(f"Set dict key '{key}' on parent model '{parent_model.__class__.__name__}' with value '{value}'")
 
+                # Now adjust our variable accounting to set the whole dict back on the parent model,
+                # allowing us to trigger any validation
+                key = part
+                value = model_to_set
+                model_to_set = parent_model
+            elif isinstance(model_to_set, list):
+                if value is None:
+                    continue
+
+                # We allow setting list values directly
+                # Grab the list from the parent model, set the value, and continue
+                model_to_set[int(key)] = value
+
+                _log.debug(f"Set list index '{key}' on parent model '{parent_model.__class__.__name__}' with value '{value}'")
+
                 # Now adjust our variable accounting to set the whole dict back on the parent model,
                 # allowing us to trigger any validation
                 key = part
@@ -427,46 +467,44 @@ def parse_extra_args_model(model: "BaseModel"):
         field = model_to_set.__class__.model_fields[key]
         adapter = TypeAdapter(field.annotation)
 
-        _log.debug(f"Setting field '{key}' on model '{model_to_set.__class__.__name__}' with raw value '{value}'")
-
-        # Convert the value using the type adapter
-        if get_origin(field.annotation) in (list, List):
-            value = value or ""
-            if isinstance(value, list):
-                # Already a list, use as is
-                pass
-            elif isinstance(value, str):
-                # Convert from comma-separated values
-                value = value.split(",")
-            else:
-                # Unknown, raise
-                raise ValueError(f"Cannot convert value '{value}' to list for field '{key}'")
-        elif get_origin(field.annotation) in (dict, Dict):
-            value = value or ""
-            if isinstance(value, dict):
-                # Already a dict, use as is
-                pass
-            elif isinstance(value, str):
-                # Convert from comma-separated key=value pairs
-                dict_items = value.split(",")
-                dict_value = {}
-                for item in dict_items:
-                    if item:
-                        k, v = item.split("=", 1)
-                        # If the key type is an enum, convert
-                        dict_value[k] = v
-
-                # Grab any previously existing dict to preserve other keys
-                existing_dict = getattr(model_to_set, key, {}) or {}
-                _log.debug(f"Existing dict for field '{key}': {existing_dict}")
-                _log.debug(f"New dict items for field '{key}': {dict_value}")
-                dict_value.update(existing_dict)
-                value = dict_value
-            else:
-                # Unknown, raise
-                raise ValueError(f"Cannot convert value '{value}' to dict for field '{key}'")
-        try:
-            if value is not None:
+        if value is not None:
+            _log.debug(f"Setting field '{key}' on model '{model_to_set.__class__.__name__}' with raw value '{value}'")
+
+            # Convert the value using the type adapter
+            if get_origin(field.annotation) in (list, List):
+                if isinstance(value, list):
+                    # Already a list, use as is
+                    pass
+                elif isinstance(value, str):
+                    # Convert from comma-separated values
+                    value = value.split(",")
+                else:
+                    # Unknown, raise
+                    raise ValueError(f"Cannot convert value '{value}' to list for field '{key}'")
+            elif get_origin(field.annotation) in (dict, Dict):
+                if isinstance(value, dict):
+                    # Already a dict, use as is
+                    pass
+                elif isinstance(value, str):
+                    # Convert from comma-separated key=value pairs
+                    dict_items = value.split(",")
+                    dict_value = {}
+                    for item in dict_items:
+                        if item:
+                            k, v = item.split("=", 1)
+                            # If the key type is an enum, convert
+                            dict_value[k] = v
+
+                    # Grab any previously existing dict to preserve other keys
+                    existing_dict = getattr(model_to_set, key, {}) or {}
+                    _log.debug(f"Existing dict for field '{key}': {existing_dict}")
+                    _log.debug(f"New dict items for field '{key}': {dict_value}")
+                    dict_value.update(existing_dict)
+                    value = dict_value
+                else:
+                    # Unknown, raise
+                    raise ValueError(f"Cannot convert value '{value}' to dict for field '{key}'")
+            try:
                 # Post process and convert keys if needed
                 # pydantic shouldve done this automatically, but alas
                 if isinstance(value, dict) and get_args(field.annotation):
@@ -482,10 +520,11 @@ def parse_extra_args_model(model: "BaseModel"):
 
                 # Set the value on the model
                 setattr(model_to_set, key, value)
-
-        except ValidationError:
-            _log.warning(f"Failed to validate field '{key}' with value '{value}' for model '{model_to_set.__class__.__name__}'")
-            continue
+            except ValidationError:
+                _log.warning(f"Failed to validate field '{key}' with value '{value}' for model '{model_to_set.__class__.__name__}'")
+                continue
+        else:
+            _log.debug(f"Skipping setting field '{key}' on model '{model_to_set.__class__.__name__}' with None value")
 
     return model, kwargs
 

hatch_build/tests/test_cli_model.py

@@ -21,6 +21,7 @@ class SubModel(BaseModel, validate_assignment=True):
     sub_arg: int = 42
     sub_arg_with_value: str = "sub_default"
     sub_arg_enum: MyEnum = MyEnum.OPTION_A
+    sub_arg_literal: Literal["x", "y", "z"] = "x"
 
 
 class MyTopLevelModel(BaseModel, validate_assignment=True):
@@ -35,25 +36,30 @@ class MyTopLevelModel(BaseModel, validate_assignment=True):
     dict_arg_default_values: Dict[str, str] = {"existing-key": "existing-value"}
     path_arg: Path = Path(".")
 
+    list_literal: List[Literal["a", "b", "c"]] = ["a"]
+    dict_literal_key: Dict[Literal["a", "b", "c"], str] = {"a": "first"}
+    dict_literal_value: Dict[str, Literal["a", "b", "c"]] = {"first": "a"}
+
     list_enum: List[MyEnum] = [MyEnum.OPTION_A]
     dict_enum: Dict[str, MyEnum] = {"first": MyEnum.OPTION_A}
     dict_enum_key: Dict[MyEnum, str] = {MyEnum.OPTION_A: "first"}
     dict_enum_key_model_value: Dict[MyEnum, SubModel] = {MyEnum.OPTION_A: SubModel()}
 
     submodel: SubModel
-    submodel2: SubModel = SubModel()
+    submodel2: SubModel = SubModel(sub_args=84, sub_arg_with_value="predefined", sub_arg_enum=MyEnum.OPTION_B, sub_arg_literal="z")
     submodel3: Optional[SubModel] = None
 
-    submodel_list: List[SubModel] = []
     submodel_list_instanced: List[SubModel] = [SubModel()]
-    submodel_dict: Dict[str, SubModel] = {}
     submodel_dict_instanced: Dict[str, SubModel] = {"a": SubModel()}
 
     unsupported_literal: Literal[b"test"] = b"test"
     unsupported_dict: Dict[SubModel, str] = {}
     unsupported_dict_mixed_types: Dict[str, Union[str, SubModel]] = {}
     unsupported_random_type: Optional[set] = None
 
+    unsupported_submodel_list: List[SubModel] = []
+    unsupported_submodel_dict: Dict[str, SubModel] = {}
+
 
 class TestCLIMdel:
     def test_get_arg_from_model(self):
@@ -81,6 +87,12 @@ def test_get_arg_from_model(self):
                     "new-value",
                     "--path-arg",
                     "/some/path",
+                    "--list-literal",
+                    "a,b",
+                    "--dict-literal-key.a",
+                    "first",
+                    "--dict-literal-value.first",
+                    "a",
                     "--list-enum",
                     "option_a,option_b",
                     "--dict-enum.first",
@@ -95,6 +107,10 @@ def test_get_arg_from_model(self):
                     "100",
                     "--submodel.sub-arg-with-value",
                     "sub_value",
+                    "--submodel.sub-arg-enum",
+                    "option_a",
+                    "--submodel.sub-arg-literal",
+                    "y",
                     "--submodel2.sub-arg",
                     "200",
                     "--submodel2.sub-arg-with-value",
@@ -103,6 +119,12 @@ def test_get_arg_from_model(self):
                     "300",
                     "--submodel-list-instanced.0.sub-arg",
                     "400",
+                    "--submodel-list-instanced.0.sub-arg-with-value",
+                    "list_value",
+                    "--submodel-list-instanced.0.sub-arg-enum",
+                    "option_b",
+                    "--submodel-list-instanced.0.sub-arg-literal",
+                    "z",
                     "--submodel-dict-instanced.a.sub-arg",
                     "500",
                 ],
@@ -127,25 +149,37 @@ def test_get_arg_from_model(self):
         assert model.dict_arg_default_values == {"existing-key": "new-value"}
         assert model.path_arg == Path("/some/path")
 
+        assert model.list_literal == ["a", "b"]
+        assert model.dict_literal_key == {"a": "first"}
+        assert model.dict_literal_value == {"first": "a"}
+
         assert model.list_enum == [MyEnum.OPTION_A, MyEnum.OPTION_B]
         assert model.dict_enum == {"first": MyEnum.OPTION_B}
         assert model.dict_enum_key == {MyEnum.OPTION_A: "first", MyEnum.OPTION_B: "second", MyEnum.OPTION_C: "third"}
         assert model.dict_enum_key_model_value[MyEnum.OPTION_A].sub_arg == 600
 
         assert model.submodel.sub_arg == 100
         assert model.submodel.sub_arg_with_value == "sub_value"
+        assert model.submodel.sub_arg_enum == MyEnum.OPTION_A
+        assert model.submodel.sub_arg_literal == "y"
         assert model.submodel2.sub_arg == 200
         assert model.submodel2.sub_arg_with_value == "sub_value2"
+        assert model.submodel2.sub_arg_enum == MyEnum.OPTION_B
+        assert model.submodel2.sub_arg_literal == "z"
+
         assert model.submodel3.sub_arg == 300
         assert model.submodel_list_instanced[0].sub_arg == 400
+        assert model.submodel_list_instanced[0].sub_arg_with_value == "list_value"
+        assert model.submodel_list_instanced[0].sub_arg_enum == MyEnum.OPTION_B
+        assert model.submodel_list_instanced[0].sub_arg_literal == "z"
         assert model.submodel_dict_instanced["a"].sub_arg == 500
 
         stderr = mock_stderr.getvalue()
         for text in (
             f"[sdist]\ndist/hatch_build-{__version__}.tar.gz",
             f"[wheel]\ndist/hatch_build-{__version__}-py3-none-any.whl",
-            "[hatch_build.cli][WARNING]: Only lists of str, int, float, or bool are supported - field `submodel_list` got <class 'test_cli_model.SubModel'>",
-            "[hatch_build.cli][WARNING]: Only dicts with str, int, float, bool, or enum values are supported - field `submodel_dict` got value type <class 'test_cli_model.SubModel'>",
+            "[hatch_build.cli][WARNING]: Only lists of str, int, float, or bool are supported - field `unsupported_submodel_list` got <class 'test_cli_model.SubModel'>",
+            "[hatch_build.cli][WARNING]: Only dicts with str, int, float, bool, or enum values are supported - field `unsupported_submodel_dict` got value type <class 'test_cli_model.SubModel'>",
             "[hatch_build.cli][WARNING]: Only Literal types of str, int, float, or bool are supported - field `unsupported_literal` got (b'test',)",
             "[hatch_build.cli][WARNING]: Only dicts with str, int, float, bool, or enum keys are supported - field `unsupported_dict` got key type <class 'test_cli_model.SubModel'>",
             "[hatch_build.cli][WARNING]: Only dicts with str, int, float, bool, or enum values are supported - field `unsupported_dict_mixed_types` got value type typing.Union[str, test_cli_model.SubModel]",