Skip to content

Conversation

@ethanbwaite
Copy link
Contributor

Summary:
As title, supports list[str] and dict[str, str] types in runopt.

Updated testcases to better cover potential type casting issues.

Differential Revision: D78767495

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 23, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D78767495

help: str

@property
def is_type_list(self) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the property be renamed to 'is_type_list_of_str' since the check ensure the elements are of type str?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, updated

return self.opt_type in (List[str], list[str])

@property
def is_type_dict(self) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to is_type_list, should this be more explicitly named as type_dict_of_str?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, updated

…r,str] (#1093)

Summary:

As title, supports `list[str]` and `dict[str, str]` types in runopt.

Updated testcases to better cover potential type casting issues.

Differential Revision: D78767495
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D78767495

Copy link
Contributor

@tonykao8080 tonykao8080 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@facebook-github-bot facebook-github-bot merged commit dc70d90 into main Jul 25, 2025
23 of 24 checks passed
@facebook-github-bot facebook-github-bot deleted the export-D78767495 branch July 25, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants