Skip to content

Commit 68d7604

Browse files
author
cobyfrombrooklyn-bot
committed
fix: show full type for Optional and generic type annotations in help
The help text for arguments with generic type annotations like Optional[str], List[int], or Union[int, str] only showed the base type name (e.g., 'Optional' or 'Union') without the type arguments. This was because _GetArgType used __qualname__ which doesn't include generic type arguments. Additionally, Optional[str] with default=None was double-wrapped as 'Optional[Optional]' or 'Optional[Union]' because the code always wrapped in Optional when default was None, even if the annotation already indicated optionality. Changes: - _GetArgType now checks for __args__ (present on generic types) and uses repr() instead of __qualname__ to preserve type arguments - The Optional wrapping now checks if 'None' is already in the type string to avoid double-wrapping Fixes #508
1 parent 716bbc2 commit 68d7604

File tree

3 files changed

+45
-1
lines changed

3 files changed

+45
-1
lines changed

fire/helptext.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,8 @@ def _CreateFlagItem(flag, docstring_info, spec, required=False,
491491

492492
# We need to handle the case where there is a default of None, but otherwise
493493
# the argument has another type.
494-
if arg_default == 'None':
494+
# Don't wrap in Optional if the type already indicates it's optional.
495+
if arg_default == 'None' and arg_type and 'None' not in arg_type:
495496
arg_type = f'Optional[{arg_type}]'
496497

497498
arg_type = f'Type: {arg_type}' if arg_type else ''
@@ -524,6 +525,11 @@ def _GetArgType(arg, spec):
524525
"""
525526
if arg in spec.annotations:
526527
arg_type = spec.annotations[arg]
528+
# For generic types (e.g., Optional[str], List[int], Union[int, str]),
529+
# __qualname__ only returns the base name without type arguments.
530+
# Use repr() instead to get the full type string.
531+
if hasattr(arg_type, '__args__'):
532+
return repr(arg_type)
527533
try:
528534
return arg_type.__qualname__
529535
except AttributeError:

fire/helptext_test.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,37 @@ def testHelpTextFunctionWithTypesAndDefaultNone(self):
152152
help_screen)
153153
self.assertNotIn('NOTES', help_screen)
154154

155+
def testHelpTextOptionalTypeWithDefault(self):
156+
# Regression test for https://github.com/google/python-fire/issues/508
157+
# Optional[str] with a non-None default should show the full type
158+
# including the inner type, not just "Optional" or "Union".
159+
component = tc.py3.WithDefaultsAndTypes().get_optional_str
160+
help_screen = helptext.HelpText(
161+
component=component,
162+
trace=trace.FireTrace(component, name='get_optional_str'))
163+
self.assertIn('Type:', help_screen)
164+
# The type annotation must include 'str' to indicate the actual type.
165+
# Without the fix, __qualname__ returns just 'Optional' or 'Union'
166+
# which loses the inner type information.
167+
self.assertRegex(help_screen, r'Type:.*str')
168+
# Must not show bare 'Optional' or 'Union' without type arguments
169+
self.assertNotRegex(help_screen, r'Type: Optional\n')
170+
self.assertNotRegex(help_screen, r'Type: Union\n')
171+
172+
def testHelpTextOptionalTypeWithNoneDefault(self):
173+
# Regression test for https://github.com/google/python-fire/issues/508
174+
# Optional[str] with default=None should NOT show "Optional[Optional]"
175+
# or "Optional[Union]".
176+
component = tc.py3.WithDefaultsAndTypes().get_optional_str_none
177+
help_screen = helptext.HelpText(
178+
component=component,
179+
trace=trace.FireTrace(component, name='get_optional_str_none'))
180+
self.assertIn('Type:', help_screen)
181+
self.assertNotIn('Optional[Optional]', help_screen)
182+
self.assertNotIn('Optional[Union]', help_screen)
183+
# Should show the inner type (str) in the type annotation
184+
self.assertRegex(help_screen, r'Type:.*str')
185+
155186
def testHelpTextFunctionWithTypes(self):
156187
component = tc.py3.WithTypes().double
157188
help_screen = helptext.HelpText(

fire/test_components_py3.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import asyncio
1818
import functools
19+
import typing
1920
from typing import Tuple
2021

2122

@@ -99,3 +100,9 @@ def double(self, count: float = 0) -> float:
99100

100101
def get_int(self, value: int = None):
101102
return 0 if value is None else value
103+
104+
def get_optional_str(self, value: typing.Optional[str] = 'something'):
105+
return value
106+
107+
def get_optional_str_none(self, value: typing.Optional[str] = None):
108+
return value

0 commit comments

Comments
 (0)