diff --git a/src/tyro/_parsers.py b/src/tyro/_parsers.py index 3b8313a8..329bc04b 100644 --- a/src/tyro/_parsers.py +++ b/src/tyro/_parsers.py @@ -240,15 +240,26 @@ def from_callable_or_type( class_field_name = _strings.make_field_name( [intern_prefix, field.intern_name] ) - if field.helptext is not None: - # Keep lazy - don't evaluate yet. - helptext_from_intern_prefixed_field_name[class_field_name] = ( - field.helptext - ) - else: - helptext_from_intern_prefixed_field_name[class_field_name] = ( - _docstrings.get_callable_description(nested_parser.f) - ) + + # `field.helptext` can be a callable that returns `None` (for + # example, lazy docstring parsing when a field has no attribute + # docstring), so the docstring fallback needs to happen after + # evaluation. We keep this lazy - don't evaluate yet. + def evaluate_group_helptext( + helptext: str | Callable[[], str | None] | None = field.helptext, + f: Callable = nested_parser.f, + ) -> str | None: + out = helptext() if callable(helptext) else helptext + if out is None: + # Fall back to the docstring of the nested type. An + # explicit empty string (`tyro.conf.arg(help="")`) + # suppresses this fallback. + out = _docstrings.get_callable_description(f) + return out + + helptext_from_intern_prefixed_field_name[class_field_name] = ( + evaluate_group_helptext + ) # If arguments are in an optional group, it indicates that the default_instance # will be used if none of the arguments are passed in. @@ -276,9 +287,11 @@ def from_callable_or_type( ) if callable(desc): desc = desc() - # If still None after evaluation, use empty string. + # A lazy description can evaluate to None, for example when a nested + # field has no attribute docstring. Fall back to the docstring of the + # type itself. An explicit empty string suppresses this fallback. if desc is None: - desc = "" + desc = _docstrings.get_callable_description(f) parser_spec = ParserSpecification( f=f, diff --git a/tests/test_helptext.py b/tests/test_helptext.py index 5df2447d..3ac035e5 100644 --- a/tests/test_helptext.py +++ b/tests/test_helptext.py @@ -1687,3 +1687,123 @@ class FreshForNoPydantic: sys.modules.pop("pydantic", None) description = _docstrings.get_callable_description(FreshForNoPydantic) assert "Docstring for FreshForNoPydantic." in description + + +def test_nested_group_docstring_as_description() -> None: + """Class docstrings of nested types should be used as the default group + description. https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupFromDocstring: + """Settings related to workflow execution.""" + + cores: int = 1 + + @dataclasses.dataclass + class ParentOfDocstringGroup: + work: NestedGroupFromDocstring = dataclasses.field( + default_factory=NestedGroupFromDocstring + ) + + helptext = get_helptext_with_checks(ParentOfDocstringGroup) + assert "Settings related to workflow execution." in helptext + + +def test_nested_group_attribute_docstring_takes_precedence() -> None: + """An attribute docstring on the field should override the nested type's + class docstring. https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupWithClassDoc: + """Class-level description.""" + + cores: int = 1 + + @dataclasses.dataclass + class ParentWithAttributeDoc: + work: NestedGroupWithClassDoc = dataclasses.field( + default_factory=NestedGroupWithClassDoc + ) + """Attribute-level description.""" + + helptext = get_helptext_with_checks(ParentWithAttributeDoc) + assert "Attribute-level description." in helptext + assert "Class-level description." not in helptext + + +def test_nested_group_arg_help_overrides_docstring() -> None: + """`tyro.conf.arg(help=...)` should override the nested type's class + docstring. https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupForArgHelp: + """Class-level description.""" + + cores: int = 1 + + @dataclasses.dataclass + class ParentWithArgHelp: + work: Annotated[ + NestedGroupForArgHelp, tyro.conf.arg(help="Explicit description.") + ] = dataclasses.field(default_factory=NestedGroupForArgHelp) + + helptext = get_helptext_with_checks(ParentWithArgHelp) + assert "Explicit description." in helptext + assert "Class-level description." not in helptext + + +def test_nested_group_arg_help_empty_suppresses_docstring() -> None: + """An explicit `tyro.conf.arg(help="")` should suppress the nested type's + class docstring. https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupForEmptyHelp: + """Class-level description.""" + + cores: int = 1 + + @dataclasses.dataclass + class ParentWithEmptyHelp: + work: Annotated[NestedGroupForEmptyHelp, tyro.conf.arg(help="")] = ( + dataclasses.field(default_factory=NestedGroupForEmptyHelp) + ) + + helptext = get_helptext_with_checks(ParentWithEmptyHelp) + assert "Class-level description." not in helptext + + +def test_nested_group_no_docstring() -> None: + """A nested dataclass without a docstring should not leak the + autogenerated `Name(field: type)` docstring into the group description. + https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupNoDocstring: + cores: int = 1 + + @dataclasses.dataclass + class ParentOfNoDocstringGroup: + work: NestedGroupNoDocstring = dataclasses.field( + default_factory=NestedGroupNoDocstring + ) + + helptext = get_helptext_with_checks(ParentOfNoDocstringGroup) + assert "NestedGroupNoDocstring(" not in helptext + + +def test_nested_group_docstring_required_field() -> None: + """The class docstring fallback should also apply when the nested field is + required. https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupRequired: + """Settings for a required group.""" + + cores: int + + @dataclasses.dataclass + class ParentOfRequiredGroup: + work: NestedGroupRequired + + helptext = get_helptext_with_checks(ParentOfRequiredGroup) + assert "Settings for a required group." in helptext diff --git a/tests/test_py311_generated/test_helptext_generated.py b/tests/test_py311_generated/test_helptext_generated.py index 1d57200a..5ad0b346 100644 --- a/tests/test_py311_generated/test_helptext_generated.py +++ b/tests/test_py311_generated/test_helptext_generated.py @@ -1687,3 +1687,123 @@ class FreshForNoPydantic: sys.modules.pop("pydantic", None) description = _docstrings.get_callable_description(FreshForNoPydantic) assert "Docstring for FreshForNoPydantic." in description + + +def test_nested_group_docstring_as_description() -> None: + """Class docstrings of nested types should be used as the default group + description. https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupFromDocstring: + """Settings related to workflow execution.""" + + cores: int = 1 + + @dataclasses.dataclass + class ParentOfDocstringGroup: + work: NestedGroupFromDocstring = dataclasses.field( + default_factory=NestedGroupFromDocstring + ) + + helptext = get_helptext_with_checks(ParentOfDocstringGroup) + assert "Settings related to workflow execution." in helptext + + +def test_nested_group_attribute_docstring_takes_precedence() -> None: + """An attribute docstring on the field should override the nested type's + class docstring. https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupWithClassDoc: + """Class-level description.""" + + cores: int = 1 + + @dataclasses.dataclass + class ParentWithAttributeDoc: + work: NestedGroupWithClassDoc = dataclasses.field( + default_factory=NestedGroupWithClassDoc + ) + """Attribute-level description.""" + + helptext = get_helptext_with_checks(ParentWithAttributeDoc) + assert "Attribute-level description." in helptext + assert "Class-level description." not in helptext + + +def test_nested_group_arg_help_overrides_docstring() -> None: + """`tyro.conf.arg(help=...)` should override the nested type's class + docstring. https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupForArgHelp: + """Class-level description.""" + + cores: int = 1 + + @dataclasses.dataclass + class ParentWithArgHelp: + work: Annotated[ + NestedGroupForArgHelp, tyro.conf.arg(help="Explicit description.") + ] = dataclasses.field(default_factory=NestedGroupForArgHelp) + + helptext = get_helptext_with_checks(ParentWithArgHelp) + assert "Explicit description." in helptext + assert "Class-level description." not in helptext + + +def test_nested_group_arg_help_empty_suppresses_docstring() -> None: + """An explicit `tyro.conf.arg(help="")` should suppress the nested type's + class docstring. https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupForEmptyHelp: + """Class-level description.""" + + cores: int = 1 + + @dataclasses.dataclass + class ParentWithEmptyHelp: + work: Annotated[NestedGroupForEmptyHelp, tyro.conf.arg(help="")] = ( + dataclasses.field(default_factory=NestedGroupForEmptyHelp) + ) + + helptext = get_helptext_with_checks(ParentWithEmptyHelp) + assert "Class-level description." not in helptext + + +def test_nested_group_no_docstring() -> None: + """A nested dataclass without a docstring should not leak the + autogenerated `Name(field: type)` docstring into the group description. + https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupNoDocstring: + cores: int = 1 + + @dataclasses.dataclass + class ParentOfNoDocstringGroup: + work: NestedGroupNoDocstring = dataclasses.field( + default_factory=NestedGroupNoDocstring + ) + + helptext = get_helptext_with_checks(ParentOfNoDocstringGroup) + assert "NestedGroupNoDocstring(" not in helptext + + +def test_nested_group_docstring_required_field() -> None: + """The class docstring fallback should also apply when the nested field is + required. https://github.com/brentyi/tyro/issues/483""" + + @dataclasses.dataclass + class NestedGroupRequired: + """Settings for a required group.""" + + cores: int + + @dataclasses.dataclass + class ParentOfRequiredGroup: + work: NestedGroupRequired + + helptext = get_helptext_with_checks(ParentOfRequiredGroup) + assert "Settings for a required group." in helptext diff --git a/tests/test_py311_generated/test_pydantic_generated.py b/tests/test_py311_generated/test_pydantic_generated.py index 70dfca41..fc97a3a3 100644 --- a/tests/test_py311_generated/test_pydantic_generated.py +++ b/tests/test_py311_generated/test_pydantic_generated.py @@ -263,3 +263,21 @@ class PydanticDataclassConfig: ) assert config3.in_channel == 0 assert config3.out_channel == 7 + + +def test_nested_model_docstring_as_group_description() -> None: + """Class docstrings of nested models should be used as the default group + description. https://github.com/brentyi/tyro/issues/483""" + + class DownsamplingConfig(BaseModel): + """Settings related to downsampling.""" + + ksize: int = 25 + depth: int = 100 + + class Workflow(BaseModel): + indir: pathlib.Path = pathlib.Path("/tmp") + down: DownsamplingConfig = DownsamplingConfig() + + helptext = get_helptext_with_checks(Workflow) + assert "Settings related to downsampling." in helptext diff --git a/tests/test_pydantic.py b/tests/test_pydantic.py index aa983697..b10cb7fb 100644 --- a/tests/test_pydantic.py +++ b/tests/test_pydantic.py @@ -264,3 +264,21 @@ class PydanticDataclassConfig: ) assert config3.in_channel == 0 assert config3.out_channel == 7 + + +def test_nested_model_docstring_as_group_description() -> None: + """Class docstrings of nested models should be used as the default group + description. https://github.com/brentyi/tyro/issues/483""" + + class DownsamplingConfig(BaseModel): + """Settings related to downsampling.""" + + ksize: int = 25 + depth: int = 100 + + class Workflow(BaseModel): + indir: pathlib.Path = pathlib.Path("/tmp") + down: DownsamplingConfig = DownsamplingConfig() + + helptext = get_helptext_with_checks(Workflow) + assert "Settings related to downsampling." in helptext