Fix #508: Show full type for Optional and generic type annotations in help#646
Open
cobyfrombrooklyn-bot wants to merge 1 commit intogoogle:masterfrom
Open
Fix #508: Show full type for Optional and generic type annotations in help#646cobyfrombrooklyn-bot wants to merge 1 commit intogoogle:masterfrom
cobyfrombrooklyn-bot wants to merge 1 commit intogoogle:masterfrom
Conversation
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 google#508
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The help text for arguments with generic type annotations like
Optional[str],List[int], orUnion[int, str]only showed the base type name (e.g.,Type: OptionalorType: Union) without the type arguments. This happened because_GetArgTypeused__qualname__which strips generic type parameters.Additionally,
Optional[str]withdefault=Nonewas double-wrapped asType: Optional[Optional]orType: Optional[Union].Changes
fire/helptext.py:_GetArgTypenow checks for__args__(present on generic types likeOptional[str],List[int]) and usesrepr()instead of__qualname__to preserve type argumentsfire/helptext.py: The Optional-wrapping logic now checks ifNoneis already in the type string to avoid double-wrappingfire/test_components_py3.py: Addedget_optional_strandget_optional_str_nonetest methodsfire/helptext_test.py: Added two tests:testHelpTextOptionalTypeWithDefault: VerifiesOptional[str]with non-None default showsstrin typetestHelpTextOptionalTypeWithNoneDefault: Verifies noOptional[Optional]orOptional[Union]double-wrappingTest
Both new tests fail without the fix and pass with it. Full test suite: 262 passed, 0 failed.
Tested locally on macOS ARM (Apple Silicon), Python 3.14.
Fixes #508