Skip to content

Commit

Permalink
Fix argument parsing bug
Browse files Browse the repository at this point in the history
Summary: Ran into an error when I had a component function with an optional bool argument that was being set to None; realized that the unit tests had the function defined but didn't actually use it yet.

Differential Revision: D48384586

fbshipit-source-id: 6b681b50ff55857e9fabfff2d6ca204d27e89a40
  • Loading branch information
kunalb authored and facebook-github-bot committed Aug 16, 2023
1 parent 744706b commit d0da3dc
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion torchx/specs/builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def materialize_appdef(
parameter_type = parameter.annotation
parameter_type = decode_optional(parameter_type)
if is_bool(parameter_type):
arg_value = arg_value.lower() == "true"
arg_value = arg_value and arg_value.lower() == "true"
elif not is_primitive(parameter_type):
arg_value = decode_from_string(arg_value, parameter_type)
if parameter.kind == inspect.Parameter.VAR_POSITIONAL:
Expand Down
4 changes: 2 additions & 2 deletions torchx/specs/test/builders_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def test_fn_with_bool(flag: bool = False) -> AppDef:
return get_dummy_application("trainer-without-flag")


def test_fn_with_bool_opional(flag: Optional[bool] = None) -> AppDef:
def test_fn_with_bool_optional(flag: Optional[bool] = None) -> AppDef:
"""Dummy app with or without flag
Args:
Expand Down Expand Up @@ -309,7 +309,7 @@ def test_bool_false(self) -> None:

def test_bool_none(self) -> None:
app_def = materialize_appdef(
test_fn_with_bool,
test_fn_with_bool_optional,
[],
)
self.assertEqual("trainer-without-flag", app_def.roles[0].name)
Expand Down

0 comments on commit d0da3dc

Please sign in to comment.