From 4721f3142aa3fb888a47c05e8006ab61d57cee11 Mon Sep 17 00:00:00 2001 From: Tim Paine <3105306+timkpaine@users.noreply.github.com> Date: Sun, 9 Nov 2025 17:35:25 -0500 Subject: [PATCH] Add fix for default args vs predefined args --- hatch_build/cli.py | 137 ++++++++++++++++++---------- hatch_build/tests/test_cli_model.py | 44 ++++++++- 2 files changed, 127 insertions(+), 54 deletions(-) diff --git a/hatch_build/cli.py b/hatch_build/cli.py index b198ae0..88a0833 100644 --- a/hatch_build/cli.py +++ b/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 diff --git a/hatch_build/tests/test_cli_model.py b/hatch_build/tests/test_cli_model.py index f4141a5..67eb14d 100644 --- a/hatch_build/tests/test_cli_model.py +++ b/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,18 +36,20 @@ 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" @@ -54,6 +57,9 @@ class MyTopLevelModel(BaseModel, validate_assignment=True): 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,6 +149,10 @@ 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"} @@ -134,18 +160,26 @@ def test_get_arg_from_model(self): 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 ", - "[hatch_build.cli][WARNING]: Only dicts with str, int, float, bool, or enum values are supported - field `submodel_dict` got value type ", + "[hatch_build.cli][WARNING]: Only lists of str, int, float, or bool are supported - field `unsupported_submodel_list` got ", + "[hatch_build.cli][WARNING]: Only dicts with str, int, float, bool, or enum values are supported - field `unsupported_submodel_dict` got value type ", "[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 ", "[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]",