Fix list[dict] regression (#228)

* Fix `list[dict]` edge case

* Bump version

* Take UseAppendAction into account

* ruff

* docs sync

* Fix UseAppendAction properly

* cast for mypy

* ruff

* nits

Author: brentyi

docs/source/examples/custom_constructors.rst

@@ -343,13 +343,13 @@ structs.
                     name="lower",
                     type=int,
                     default=default[0],
-                    helptext="Lower bound." "",
+                    helptext="Lower bound.",
                 ),
                 tyro.constructors.StructFieldSpec(
                     name="upper",
                     type=int,
                     default=default[1],
-                    helptext="Upper bound." "",
+                    helptext="Upper bound.",
                 ),
             ),
         )

examples/06_custom_constructors/04_struct_registry.py

@@ -60,13 +60,13 @@ def _(
                 name="lower",
                 type=int,
                 default=default[0],
-                helptext="Lower bound." "",
+                helptext="Lower bound.",
             ),
             tyro.constructors.StructFieldSpec(
                 name="upper",
                 type=int,
                 default=default[1],
-                helptext="Upper bound." "",
+                helptext="Upper bound.",
             ),
         ),
     )

src/tyro/__init__.py

@@ -1,6 +1,6 @@
 from typing import TYPE_CHECKING
 
-__version__ = "0.9.7"
+__version__ = "0.9.8"
 
 
 from . import conf as conf

src/tyro/_fields.py

@@ -182,10 +182,7 @@ def is_positional_call(self) -> bool:
 def is_struct_type(typ: Union[TypeForm[Any], Callable], default_instance: Any) -> bool:
     """Determine whether a type should be treated as a 'struct type', where a single
     type can be broken down into multiple fields (eg for nested dataclasses or
-    classes).
-
-    TODO: we should come up with a better name than 'struct type', which is a little bit
-    misleading."""
+    classes)."""
 
     list_or_error = field_list_from_type_or_callable(
         typ, default_instance, support_single_arg_types=False

src/tyro/_parsers.py

@@ -373,7 +373,7 @@ def handle_field(
 
         # (2) Handle nested callables.
         if force_primitive == "struct" or _fields.is_struct_type(
-            field.type_stripped, field.default
+            field.type, field.default
         ):
             field = field.with_new_type_stripped(
                 _resolver.narrow_subtypes(

src/tyro/conf/_confstruct.py

@@ -116,9 +116,9 @@ def subcommand(
             will be used in place of the argument's type for parsing arguments.
             For more configurability, see :mod:`tyro.constructors`.
     """
-    assert not (
-        constructor is not None and constructor_factory is not None
-    ), "`constructor` and `constructor_factory` cannot both be set."
+    assert not (constructor is not None and constructor_factory is not None), (
+        "`constructor` and `constructor_factory` cannot both be set."
+    )
     return _SubcommandConfig(
         name,
         default,
@@ -220,9 +220,9 @@ def arg(
     Returns:
         Object to attach via `typing.Annotated[]`.
     """
-    assert not (
-        constructor is not None and constructor_factory is not None
-    ), "`constructor` and `constructor_factory` cannot both be set."
+    assert not (constructor is not None and constructor_factory is not None), (
+        "`constructor` and `constructor_factory` cannot both be set."
+    )
 
     if aliases is not None:
         for alias in aliases:

src/tyro/constructors/_primitive_spec.py

@@ -331,11 +331,15 @@ def sequence_rule(
         if container_type is collections.abc.Sequence:
             container_type = list
 
+        args = get_args(type_info.type)
         if container_type is tuple:
-            (contained_type, ell) = get_args(type_info.type)
+            assert len(args) == 2
+            (contained_type, ell) = args
             assert ell == Ellipsis
+        elif len(args) == 1:
+            (contained_type,) = args
         else:
-            (contained_type,) = get_args(type_info.type)
+            contained_type = Any
 
         inner_spec = ConstructorRegistry.get_primitive_spec(
             PrimitiveTypeInfo.make(
@@ -687,7 +691,9 @@ def str_from_instance(instance: Any) -> List[str]:
             if fuzzy_match is not None:
                 return fuzzy_match.str_from_instance(instance)
 
-            assert False, f"could not match default value {instance} with any types in union {options}"
+            assert False, (
+                f"could not match default value {instance} with any types in union {options}"
+            )
 
         return PrimitiveConstructorSpec(
             nargs=nargs,

src/tyro/constructors/_struct_spec.py

@@ -244,9 +244,9 @@ def typeddict_rule(info: StructTypeInfo) -> StructConstructorSpec | None:
 
             if typ_origin in (Required, NotRequired):
                 args = get_args(typ)
-                assert (
-                    len(args) == 1
-                ), "typing.Required[] and typing.NotRequired[T] require a concrete type T."
+                assert len(args) == 1, (
+                    "typing.Required[] and typing.NotRequired[T] require a concrete type T."
+                )
                 typ = args[0]
                 del args
 
@@ -377,7 +377,9 @@ def namedtuple_rule(info: StructTypeInfo) -> StructConstructorSpec | None:
         return StructConstructorSpec(instantiate=info.type, fields=tuple(field_list))
 
     @registry.struct_rule
-    def sequence_rule(info: StructTypeInfo) -> StructConstructorSpec | None:
+    def variable_length_sequence_rule(
+        info: StructTypeInfo,
+    ) -> StructConstructorSpec | None:
         if get_origin(info.type) not in (
             list,
             set,
@@ -387,13 +389,34 @@ def sequence_rule(info: StructTypeInfo) -> StructConstructorSpec | None:
         ) or not isinstance(info.default, Iterable):
             return None
 
-        contained_type = get_args(info.type)[0] if get_args(info.type) else Any
+        # Cast is for mypy.
+        contained_type = cast(
+            type, get_args(info.type)[0] if get_args(info.type) else Any
+        )
 
         # If the inner type is a primitive, we'll just treat the whole type as
         # a primitive.
-        from ._registry import ConstructorRegistry
+        from ._registry import (
+            ConstructorRegistry,
+            PrimitiveConstructorSpec,
+            PrimitiveTypeInfo,
+        )
 
-        if ConstructorRegistry._is_primitive_type(contained_type, set(info.markers)):
+        contained_primitive_spec = ConstructorRegistry.get_primitive_spec(
+            PrimitiveTypeInfo.make(contained_type, set(info.markers))
+        )
+        if (
+            isinstance(contained_primitive_spec, PrimitiveConstructorSpec)
+            # Why do we check nargs?
+            # Because for primitives, we can't nest variable-length collections.
+            #
+            # For example, list[list[str]] can't be parsed as a single primitive.
+            #
+            # However, list[list[str]] can be parsed if the outer type is
+            # handled as a struct (and a default value is provided, which we
+            # check above).
+            and contained_primitive_spec.nargs != "*"
+        ):
             return None
 
         field_list = []

tests/test_collections.py

@@ -653,3 +653,56 @@ def test_list_narrowing_direct() -> None:
 def test_tuple_direct() -> None:
     assert tyro.cli(Tuple[int, ...], args="1 2".split(" ")) == (1, 2)  # type: ignore
     assert tyro.cli(Tuple[int, int], args="1 2".split(" ")) == (1, 2)  # type: ignore
+
+
+def test_nested_dict_in_list() -> None:
+    """https://github.com/nerfstudio-project/nerfstudio/pull/3567"""
+
+    @dataclasses.dataclass
+    class Args:
+        proposal_net_args_list: List[Dict] = dataclasses.field(
+            default_factory=lambda: [
+                {
+                    "hidden_dim": 16,
+                },
+                {
+                    "hidden_dim": 16,
+                },
+            ]
+        )
+        proposal_net_args_list2: Tuple[Dict[str, List], Dict[str, List]] = (
+            dataclasses.field(
+                default_factory=lambda: (
+                    {
+                        "hidden_dim": [16, 32],
+                    },
+                    {
+                        "hidden_dim": [16, 32],
+                    },
+                )
+            )
+        )
+
+    assert tyro.cli(Args, args=[]) == Args()
+    assert tyro.cli(
+        Args, args=["--proposal-net-args-list.0.hidden-dim", "32"]
+    ).proposal_net_args_list == (
+        [
+            {
+                "hidden_dim": 32,
+            },
+            {
+                "hidden_dim": 16,
+            },
+        ]
+    )
+    assert tyro.cli(
+        Args, args=["--proposal-net-args-list2.1.hidden-dim", "32", "64"]
+    ).proposal_net_args_list2 == (
+        {
+            "hidden_dim": [16, 32],
+        },
+        {
+            "hidden_dim": [32, 64],
+        },
+    )

tests/test_nested.py

@@ -282,7 +282,7 @@ class ModelSettings:
         tyro.conf.OmitSubcommandPrefixes[
             tyro.conf.ConsolidateSubcommandArgs[ModelSettings]
         ],
-        args=("output-head-settings None" " --number-of-outputs 5".split(" ")),
+        args=("output-head-settings None --number-of-outputs 5".split(" ")),
     ) == ModelSettings(OutputHeadSettings(5), None)
 
     assert tyro.cli(

tests/test_py311_generated/test_collections_generated.py

@@ -652,3 +652,56 @@ def test_list_narrowing_direct() -> None:
 def test_tuple_direct() -> None:
     assert tyro.cli(Tuple[int, ...], args="1 2".split(" ")) == (1, 2)  # type: ignore
     assert tyro.cli(Tuple[int, int], args="1 2".split(" ")) == (1, 2)  # type: ignore
+
+
+def test_nested_dict_in_list() -> None:
+    """https://github.com/nerfstudio-project/nerfstudio/pull/3567"""
+
+    @dataclasses.dataclass
+    class Args:
+        proposal_net_args_list: List[Dict] = dataclasses.field(
+            default_factory=lambda: [
+                {
+                    "hidden_dim": 16,
+                },
+                {
+                    "hidden_dim": 16,
+                },
+            ]
+        )
+        proposal_net_args_list2: Tuple[Dict[str, List], Dict[str, List]] = (
+            dataclasses.field(
+                default_factory=lambda: (
+                    {
+                        "hidden_dim": [16, 32],
+                    },
+                    {
+                        "hidden_dim": [16, 32],
+                    },
+                )
+            )
+        )
+
+    assert tyro.cli(Args, args=[]) == Args()
+    assert tyro.cli(
+        Args, args=["--proposal-net-args-list.0.hidden-dim", "32"]
+    ).proposal_net_args_list == (
+        [
+            {
+                "hidden_dim": 32,
+            },
+            {
+                "hidden_dim": 16,
+            },
+        ]
+    )
+    assert tyro.cli(
+        Args, args=["--proposal-net-args-list2.1.hidden-dim", "32", "64"]
+    ).proposal_net_args_list2 == (
+        {
+            "hidden_dim": [16, 32],
+        },
+        {
+            "hidden_dim": [32, 64],
+        },
+    )

tests/test_py311_generated/test_nested_generated.py

@@ -292,7 +292,7 @@ class ModelSettings:
         tyro.conf.OmitSubcommandPrefixes[
             tyro.conf.ConsolidateSubcommandArgs[ModelSettings]
         ],
-        args=("output-head-settings None" " --number-of-outputs 5".split(" ")),
+        args=("output-head-settings None --number-of-outputs 5".split(" ")),
     ) == ModelSettings(OutputHeadSettings(5), None)
 
     assert tyro.cli(