From 903285fbcc4e51332d562499bd90ec9a1513c813 Mon Sep 17 00:00:00 2001 From: Vineeth Sai Varikuntla Date: Wed, 16 Sep 2026 05:14:54 -0700 Subject: [PATCH] [Bugfix] Let an optional Literal flag accept the None it advertises (#52370) Signed-off-by: Vineeth Sai Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Harry Mellor <19981378+hmellor@users.noreply.github.com> --- tests/engine/test_arg_utils.py | 27 +++++++++++++++++++++++---- vllm/engine/arg_utils.py | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/engine/test_arg_utils.py b/tests/engine/test_arg_utils.py index d6ce5b36b5..e583798746 100644 --- a/tests/engine/test_arg_utils.py +++ b/tests/engine/test_arg_utils.py @@ -254,10 +254,13 @@ def test_get_type_hints(type_hint, expected): assert get_type_hints(type_hint) == expected -def test_get_kwargs(): - kwargs = get_kwargs(DummyConfig) - print(kwargs) +@pytest.fixture +def dummy_config_kwargs(): + return get_kwargs(DummyConfig) + +def test_get_kwargs(dummy_config_kwargs): + kwargs = dummy_config_kwargs # bools should not have their type set assert kwargs["regular_bool"].get("type") is None assert kwargs["optional_bool"].get("type") is None @@ -267,7 +270,7 @@ def test_get_kwargs(): assert kwargs["optional_bool_or_str"]["const"] is True assert "action" not in kwargs["optional_bool_or_str"] # optional literals should have None as a choice - assert kwargs["optional_literal"]["choices"] == ["x", "y", "None"] + assert kwargs["optional_literal"]["choices"] == ["x", "y", None] # tuples should have the correct nargs assert kwargs["tuple_n"]["nargs"] == "+" assert kwargs["tuple_2"]["nargs"] == 2 @@ -293,6 +296,22 @@ def test_get_kwargs(): assert kwargs["nested_config"]["type"]('{"field": 2}') == NestedConfig(2) # type: ignore[call-arg] +@pytest.mark.parametrize( + ("args", "expected"), + [ + (["--optional-literal", "None"], None), + (["--optional-literal", ""], None), + (["--optional-literal", "x"], "x"), + ], +) +def test_optional_handling(args, expected, dummy_config_kwargs): + parser = FlexibleArgumentParser() + parser.add_argument("--optional-literal", **dummy_config_kwargs["optional_literal"]) + + assert parser.parse_args(args).optional_literal is expected + assert "None" in parser.format_help() + + def test_jit_monitor_verbose_arg(): parser = EngineArgs.add_cli_args(FlexibleArgumentParser()) args = parser.parse_args(["--jit-monitor-verbose"]) diff --git a/vllm/engine/arg_utils.py b/vllm/engine/arg_utils.py index 6128bc6255..a977b64649 100644 --- a/vllm/engine/arg_utils.py +++ b/vllm/engine/arg_utils.py @@ -411,7 +411,7 @@ def _compute_kwargs(cls: ConfigType) -> dict[str, dict[str, Any]]: if type(None) in type_hints and not contains_type(type_hints, bool): kwargs[name]["type"] = optional_type(kwargs[name]["type"]) if kwargs[name].get("choices"): - kwargs[name]["choices"].append("None") + kwargs[name]["choices"].append(None) return kwargs @@ -1526,7 +1526,7 @@ class EngineArgs: ) # TODO: generalise this special case choices = observability_kwargs["collect_detailed_traces"]["choices"] - metavar = f"{{{','.join(choices)}}}" + metavar = f"{{{','.join(str(c) for c in choices)}}}" observability_kwargs["collect_detailed_traces"]["metavar"] = metavar observability_kwargs["collect_detailed_traces"]["choices"] += [ ",".join(p) for p in permutations(get_args(DetailedTraceModules), r=2)