Skip to content

Commit

Permalink
Improve coverage and remove unexercised logic
Browse files Browse the repository at this point in the history
  • Loading branch information
a-gardner1 committed Oct 2, 2024
1 parent 043dc67 commit 6773e0f
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 13 deletions.
5 changes: 1 addition & 4 deletions jsonargparse/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1319,10 +1319,7 @@ def _apply_actions(
keys.append(action_dest)
elif getattr(action, "jsonnet_ext_vars", False):
prev_cfg[action_dest] = value
if value == inspect._empty:
cfg.pop(action_dest, None)
else:
cfg[action_dest] = value
cfg[action_dest] = value
return cfg[parent_key] if parent_key else cfg

def merge_config(self, cfg_from: Namespace, cfg_to: Namespace) -> Namespace:
Expand Down
5 changes: 1 addition & 4 deletions jsonargparse/_parameter_resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,11 +345,8 @@ def unpack_typed_dict_kwargs(params: ParamList) -> bool:
)
)
# insert in-place
trailing_params = [] # expected to be empty
for _ in range(kwargs_idx, len(params)):
trailing_params.append(params.pop(kwargs_idx))
assert kwargs_idx == len(params), "trailing params should yield a syntax error"
params.extend(new_params)
params.extend(trailing_params)
return True
return False

Expand Down
2 changes: 0 additions & 2 deletions jsonargparse/_typehints.py
Original file line number Diff line number Diff line change
Expand Up @@ -597,8 +597,6 @@ def _check_type(self, value, append=False, cfg=None):
assert ex # needed due to ruff bug that removes " as ex"
if orig_val == "-" and isinstance(getattr(ex, "parent", None), PathError):
raise ex
if get_typehint_origin(self._typehint) in not_required_types and val == inspect._empty:
ex = None
try:
if isinstance(orig_val, str):
with change_to_path_dir(config_path):
Expand Down
37 changes: 34 additions & 3 deletions jsonargparse_tests/test_typehints.py
Original file line number Diff line number Diff line change
Expand Up @@ -617,22 +617,53 @@ def test_unpack_support(parser):
if Unpack: # and Required and NotRequired
MyTestUnpackDict = TypedDict("MyTestUnpackDict", {"a": Required[int], "b": NotRequired[int]}, total=True)

class InnerTestClass:
class UnpackClass:

def __init__(self, **kwargs: Unpack[MyTestUnpackDict]) -> None:
self.a = kwargs["a"]
self.b = kwargs.get("b")

# force fallback on AST resolver
UnpackByAssumptionsClass = type(
"UnpackByAssumptionsClass",
(UnpackClass,),
{"__module__": __name__, "__init__": lambda self, **kwargs: UnpackClass.__init__(self, **kwargs)},

Check notice

Code scanning / CodeQL

Unnecessary lambda Note test

This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
)

@dataclass
class MyTestUnpackClass:

test: InnerTestClass
test: UnpackClass

@dataclass
class MyTestInheritedUnpackClass:

test: UnpackByAssumptionsClass # type: ignore[valid-type]


@pytest.mark.skipif(not Unpack, reason="Unpack introduced in python 3.11 or backported in typing_extensions")
def test_unpack_typeddict(parser):
parser.add_argument("--testclass", type=MyTestUnpackClass)
test_config = {"test": {"class_path": f"{__name__}.InnerTestClass", "init_args": {}}}
test_config = {"test": {"class_path": f"{__name__}.UnpackClass", "init_args": {}}}
for init_args in [{"a": 1}, {"a": 2, "b": None}, {"a": 3, "b": 1}]:
test_config["test"]["init_args"] = init_args
cfg = parser.parse_args([f"--testclass={json.dumps(test_config)}"])
assert test_config == namespace_to_dict(cfg["testclass"])
# also assert no issues with dumping
if test_config["test"]["init_args"].get("b") is None:
# parser.dump does not dump null b
test_config["test"]["init_args"].pop("b", None)
assert json.dumps({"testclass": test_config}).replace(" ", "") == parser.dump(cfg, format="json")
for init_args in [{}, {"b": None}, {"b": 1}]:
with pytest.raises(ArgumentError):
test_config["test"]["init_args"] = init_args
parser.parse_args([f"--testclass={json.dumps(test_config)}"])


@pytest.mark.skipif(not Unpack, reason="Unpack introduced in python 3.11 or backported in typing_extensions")
def test_unpack_typeddict_by_assumptions(parser):
parser.add_argument("--testclass", type=MyTestInheritedUnpackClass)
test_config = {"test": {"class_path": f"{__name__}.UnpackByAssumptionsClass", "init_args": {}}}
for init_args in [{"a": 1}, {"a": 2, "b": None}, {"a": 3, "b": 1}]:
test_config["test"]["init_args"] = init_args
cfg = parser.parse_args([f"--testclass={json.dumps(test_config)}"])
Expand Down

0 comments on commit 6773e0f

Please sign in to comment.