Skip to content

Commit

Permalink
Fixup Optional runopt cfg values handling during cfg_from_json_repr d…
Browse files Browse the repository at this point in the history
…eserialization

Differential Revision: D68445341

Pull Request resolved: #1000
  • Loading branch information
lgarg26 authored Jan 21, 2025
1 parent ffb6cef commit 4789001
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
7 changes: 6 additions & 1 deletion torchx/specs/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,12 @@ def cfg_from_json_repr(self, json_repr: str) -> Dict[str, CfgVal]:
for key, val in cfg_dict.items():
runopt_ = self.get(key)
if runopt_:
if runopt_.opt_type == List[str]:
# Optional runopt cfg values default their value to None,
# but use `_type` to specify their type when provided.
# Make sure not to treat None's as lists/dictionaries
if val is None:
cfg[key] = val
elif runopt_.opt_type == List[str]:
cfg[key] = [str(v) for v in val]
elif runopt_.opt_type == Dict[str, str]:
cfg[key] = {str(k): str(v) for k, v in val.items()}
Expand Down
5 changes: 4 additions & 1 deletion torchx/specs/test/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,7 @@ def test_config_from_json_repr(self) -> None:
opts.add("disable", type_=bool, default=True, help="")
opts.add("complex_list", type_=List[str], default=[], help="")
opts.add("complex_dict", type_=Dict[str, str], default={}, help="")
opts.add("default_none", type_=List[str], help="")

self.assertDictEqual(
{
Expand All @@ -565,6 +566,7 @@ def test_config_from_json_repr(self) -> None:
"disable": False,
"complex_list": ["v1", "v2", "v3"],
"complex_dict": {"k1": "v1", "k2": "v2"},
"default_none": None,
},
opts.resolve(
opts.cfg_from_json_repr(
Expand All @@ -575,7 +577,8 @@ def test_config_from_json_repr(self) -> None:
"enable": true,
"disable": false,
"complex_list": ["v1", "v2", "v3"],
"complex_dict": {"k1": "v1", "k2": "v2"}
"complex_dict": {"k1": "v1", "k2": "v2"},
"default_none": null
}"""
)
),
Expand Down

0 comments on commit 4789001

Please sign in to comment.