From 37d67a8fcafc806174d815bb3f30eec22d8baa89 Mon Sep 17 00:00:00 2001 From: Suhwan Choi Date: Fri, 15 Aug 2025 08:33:26 +0000 Subject: [PATCH 01/14] Improve parameter kind handling for argument requirement determination - Add sophisticated parameter kind-aware logic for determining CLI argument requirements - Introduce is_option variable to separate Python semantics from CLI argument style - Ensure KEYWORD_ONLY parameters always use --flag style and are properly marked as required - Fix required attribute logic for parameters that can't be made positional - Maintain full backward compatibility while improving edge case handling --- jsonargparse/_signatures.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/jsonargparse/_signatures.py b/jsonargparse/_signatures.py index fa144690..43ca829e 100644 --- a/jsonargparse/_signatures.py +++ b/jsonargparse/_signatures.py @@ -344,7 +344,21 @@ def _add_signature_parameter( default = None elif get_typehint_origin(annotation) in not_required_types: default = SUPPRESS - is_required = default == inspect_empty + # Determine argument characteristics based on parameter kind and default value + if kind == kinds.POSITIONAL_ONLY: + is_required = True # Always required + is_option = False # Can be positional + elif kind == kinds.POSITIONAL_OR_KEYWORD: + is_required = default == inspect_empty # Required if no default + is_option = False # Can be positional + elif kind == kinds.KEYWORD_ONLY: + is_required = default == inspect_empty # Required if no default + is_option = True # Must use --flag style + elif kind in {kinds.VAR_POSITIONAL, kinds.VAR_KEYWORD}: + # These parameter types don't translate well to CLI arguments + return # Skip entirely + else: + raise ValueError(f"Unknown parameter kind: {kind}") src = get_parameter_origins(param.component, param.parent) skip_message = f'Skipping parameter "{name}" from "{src}" because of: ' if not fail_untyped and annotation == inspect_empty: @@ -356,7 +370,7 @@ def _add_signature_parameter( default = None is_required = False is_required_link_target = True - if kind in {kinds.VAR_POSITIONAL, kinds.VAR_KEYWORD} or (not is_required and name[0] == "_"): + if not is_required and name[0] == "_": return elif skip and name in skip: self.logger.debug(skip_message + "Parameter requested to be skipped.") @@ -371,12 +385,12 @@ def _add_signature_parameter( kwargs["default"] = default if default is None and not is_optional(annotation, object) and not is_required_link_target: annotation = Optional[annotation] - elif not as_positional: + elif not as_positional or is_option: kwargs["required"] = True is_subclass_typehint = False is_dataclass_like_typehint = is_dataclass_like(annotation) dest = (nested_key + "." if nested_key else "") + name - args = [dest if is_required and as_positional else "--" + dest] + args = [dest if is_required and as_positional and not is_option else "--" + dest] if param.origin: parser = container if not isinstance(container, ArgumentParser): From 32b2f8c7e0911bf6e6d717f130799985f511bde4 Mon Sep 17 00:00:00 2001 From: Suhwan Choi Date: Fri, 15 Aug 2025 08:46:32 +0000 Subject: [PATCH 02/14] Fix handling of parameters with kind=None - Add fallback case for programmatically created parameters without kind attribute - Preserves backward compatibility for add_subclass_arguments and similar methods - Fixes test failures in subclasses and link_arguments modules --- jsonargparse/_signatures.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jsonargparse/_signatures.py b/jsonargparse/_signatures.py index 43ca829e..aae2a08b 100644 --- a/jsonargparse/_signatures.py +++ b/jsonargparse/_signatures.py @@ -357,6 +357,10 @@ def _add_signature_parameter( elif kind in {kinds.VAR_POSITIONAL, kinds.VAR_KEYWORD}: # These parameter types don't translate well to CLI arguments return # Skip entirely + elif kind is None: + # Fallback for programmatically created parameters without kind + is_required = default == inspect_empty # Required if no default + is_option = False # Can be positional (preserve old behavior) else: raise ValueError(f"Unknown parameter kind: {kind}") src = get_parameter_origins(param.component, param.parent) From 2792b91acd3b99bca3d1b851cf533cc39944bdda Mon Sep 17 00:00:00 2001 From: Suhwan Choi Date: Fri, 15 Aug 2025 09:20:38 +0000 Subject: [PATCH 03/14] Update CHANGELOG.rst for PR #756 and add test coverage - Add entry for improved parameter kind handling for argument requirement determination - KEYWORD_ONLY parameters now correctly use --flag style - Add test coverage for positional-only parameter handling --- CHANGELOG.rst | 3 +++ jsonargparse_tests/test_signatures.py | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 857a9cf6..04333fc2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -22,6 +22,9 @@ Added Changed ^^^^^^^ +- Improved parameter kind handling for argument requirement determination. + ``KEYWORD_ONLY`` parameters now correctly use ``--flag`` style (`#756 + `__). - Removed support for python 3.8 (`#752 `__). - ``YAML`` comments feature is now implemented in a separate class to allow diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index da706d11..d5d8a2d1 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -697,3 +697,22 @@ def test_add_function_param_conflict(parser): with pytest.raises(ValueError) as ctx: parser.add_function_arguments(func_param_conflict) ctx.match("Unable to add parameter 'cfg' from") + + +def func_positional_only(a: int, /, b: int = 1): + """Function with positional-only parameters.""" + return a, b + + +def test_add_function_positional_only_parameters(parser): + """Test coverage for POSITIONAL_ONLY parameter handling (lines 349-350).""" + added_args = parser.add_function_arguments(func_positional_only, fail_untyped=False) + + # Both 'a' and 'b' should be added + assert "a" in added_args + assert "b" in added_args + + # Test that we can parse with both parameters + cfg = parser.parse_args(["--a=1", "--b=2"]) + assert cfg.a == 1 + assert cfg.b == 2 From 949e1cedae77262e4bce798fd45b9680a409662c Mon Sep 17 00:00:00 2001 From: Suhwan Choi Date: Sun, 17 Aug 2025 06:49:39 +0000 Subject: [PATCH 04/14] feat: rename is_option into is_non_positional --- jsonargparse/_signatures.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/jsonargparse/_signatures.py b/jsonargparse/_signatures.py index aae2a08b..84d85f08 100644 --- a/jsonargparse/_signatures.py +++ b/jsonargparse/_signatures.py @@ -347,20 +347,20 @@ def _add_signature_parameter( # Determine argument characteristics based on parameter kind and default value if kind == kinds.POSITIONAL_ONLY: is_required = True # Always required - is_option = False # Can be positional + is_non_positional = False # Can be positional elif kind == kinds.POSITIONAL_OR_KEYWORD: is_required = default == inspect_empty # Required if no default - is_option = False # Can be positional + is_non_positional = False # Can be positional elif kind == kinds.KEYWORD_ONLY: is_required = default == inspect_empty # Required if no default - is_option = True # Must use --flag style + is_non_positional = True # Must use --flag style elif kind in {kinds.VAR_POSITIONAL, kinds.VAR_KEYWORD}: # These parameter types don't translate well to CLI arguments return # Skip entirely elif kind is None: # Fallback for programmatically created parameters without kind is_required = default == inspect_empty # Required if no default - is_option = False # Can be positional (preserve old behavior) + is_non_positional = False # Can be positional (preserve old behavior) else: raise ValueError(f"Unknown parameter kind: {kind}") src = get_parameter_origins(param.component, param.parent) @@ -389,12 +389,12 @@ def _add_signature_parameter( kwargs["default"] = default if default is None and not is_optional(annotation, object) and not is_required_link_target: annotation = Optional[annotation] - elif not as_positional or is_option: + elif not as_positional or is_non_positional: kwargs["required"] = True is_subclass_typehint = False is_dataclass_like_typehint = is_dataclass_like(annotation) dest = (nested_key + "." if nested_key else "") + name - args = [dest if is_required and as_positional and not is_option else "--" + dest] + args = [dest if is_required and as_positional and not is_non_positional else "--" + dest] if param.origin: parser = container if not isinstance(container, ArgumentParser): From 82158d0f68678a7e8ce10dcca953787cde9aca6f Mon Sep 17 00:00:00 2001 From: Suhwan Choi Date: Sun, 17 Aug 2025 07:01:15 +0000 Subject: [PATCH 05/14] Move #756 to fixed in changelog --- CHANGELOG.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 04333fc2..f0271368 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -20,11 +20,14 @@ Added - Support for Python 3.14 (`#753 `__). -Changed -^^^^^^^ +Fixed +^^^^^ - Improved parameter kind handling for argument requirement determination. ``KEYWORD_ONLY`` parameters now correctly use ``--flag`` style (`#756 `__). + +Changed +^^^^^^^ - Removed support for python 3.8 (`#752 `__). - ``YAML`` comments feature is now implemented in a separate class to allow From eed540a9b0c0479dbf706827e4dc4cb3696e5e8e Mon Sep 17 00:00:00 2001 From: MilkClouds Date: Sun, 17 Aug 2025 16:01:56 +0900 Subject: [PATCH 06/14] Update jsonargparse_tests/test_signatures.py Co-authored-by: Mauricio Villegas <5780272+mauvilsa@users.noreply.github.com> --- jsonargparse_tests/test_signatures.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index d5d8a2d1..8fd314ea 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -708,10 +708,6 @@ def test_add_function_positional_only_parameters(parser): """Test coverage for POSITIONAL_ONLY parameter handling (lines 349-350).""" added_args = parser.add_function_arguments(func_positional_only, fail_untyped=False) - # Both 'a' and 'b' should be added - assert "a" in added_args - assert "b" in added_args - # Test that we can parse with both parameters cfg = parser.parse_args(["--a=1", "--b=2"]) assert cfg.a == 1 From f3d20ff95d6db78eab6580caeb210a9d07bb3f10 Mon Sep 17 00:00:00 2001 From: MilkClouds Date: Sun, 17 Aug 2025 16:03:22 +0900 Subject: [PATCH 07/14] Update jsonargparse_tests/test_signatures.py Co-authored-by: Mauricio Villegas <5780272+mauvilsa@users.noreply.github.com> --- jsonargparse_tests/test_signatures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index 8fd314ea..4ca808e1 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -706,7 +706,7 @@ def func_positional_only(a: int, /, b: int = 1): def test_add_function_positional_only_parameters(parser): """Test coverage for POSITIONAL_ONLY parameter handling (lines 349-350).""" - added_args = parser.add_function_arguments(func_positional_only, fail_untyped=False) + added_args = parser.add_function_arguments(func_positional_only) # Test that we can parse with both parameters cfg = parser.parse_args(["--a=1", "--b=2"]) From f0ebb05929fadfef7484205fcdf8ee1a2818dd95 Mon Sep 17 00:00:00 2001 From: MilkClouds Date: Sun, 17 Aug 2025 16:03:27 +0900 Subject: [PATCH 08/14] Update jsonargparse_tests/test_signatures.py Co-authored-by: Mauricio Villegas <5780272+mauvilsa@users.noreply.github.com> --- jsonargparse_tests/test_signatures.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index 4ca808e1..73aa0f47 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -705,7 +705,6 @@ def func_positional_only(a: int, /, b: int = 1): def test_add_function_positional_only_parameters(parser): - """Test coverage for POSITIONAL_ONLY parameter handling (lines 349-350).""" added_args = parser.add_function_arguments(func_positional_only) # Test that we can parse with both parameters From b308963d48acb537aba2791e27a0eafc81c6b8af Mon Sep 17 00:00:00 2001 From: MilkClouds Date: Sun, 17 Aug 2025 16:04:07 +0900 Subject: [PATCH 09/14] Update jsonargparse_tests/test_signatures.py Co-authored-by: Mauricio Villegas <5780272+mauvilsa@users.noreply.github.com> --- jsonargparse_tests/test_signatures.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index 73aa0f47..45ef9b38 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -700,7 +700,6 @@ def test_add_function_param_conflict(parser): def func_positional_only(a: int, /, b: int = 1): - """Function with positional-only parameters.""" return a, b From 247e63b93b8de1c1bea3c0d78fbdd3e0eb273143 Mon Sep 17 00:00:00 2001 From: Suhwan Choi Date: Sun, 17 Aug 2025 07:33:45 +0000 Subject: [PATCH 10/14] Fixed test to work for all argument, positional to keyword --- jsonargparse_tests/test_signatures.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index 45ef9b38..a87c1125 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -699,14 +699,16 @@ def test_add_function_param_conflict(parser): ctx.match("Unable to add parameter 'cfg' from") -def func_positional_only(a: int, /, b: int = 1): - return a, b +def func_positional_and_keyword(a: int, /, b: int, *, c: int, d: int = 1): + pass -def test_add_function_positional_only_parameters(parser): - added_args = parser.add_function_arguments(func_positional_only) +def test_add_function_positional_and_keyword_parameters(parser): + parser.add_function_arguments(func_positional_and_keyword, as_positional=True) # Test that we can parse with both parameters - cfg = parser.parse_args(["--a=1", "--b=2"]) + cfg = parser.parse_args(["1", "2", "--c=3", "--d=4"]) assert cfg.a == 1 assert cfg.b == 2 + assert cfg.c == 3 + assert cfg.d == 4 From 1f9b22e70f8f91e151c54888af13b2897a8c4c5d Mon Sep 17 00:00:00 2001 From: Suhwan Choi Date: Sun, 17 Aug 2025 07:38:24 +0000 Subject: [PATCH 11/14] Removed unreachable if in signature --- jsonargparse/_signatures.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/jsonargparse/_signatures.py b/jsonargparse/_signatures.py index 84d85f08..9547dcc7 100644 --- a/jsonargparse/_signatures.py +++ b/jsonargparse/_signatures.py @@ -357,12 +357,10 @@ def _add_signature_parameter( elif kind in {kinds.VAR_POSITIONAL, kinds.VAR_KEYWORD}: # These parameter types don't translate well to CLI arguments return # Skip entirely - elif kind is None: + else: # Fallback for programmatically created parameters without kind is_required = default == inspect_empty # Required if no default is_non_positional = False # Can be positional (preserve old behavior) - else: - raise ValueError(f"Unknown parameter kind: {kind}") src = get_parameter_origins(param.component, param.parent) skip_message = f'Skipping parameter "{name}" from "{src}" because of: ' if not fail_untyped and annotation == inspect_empty: From 752d937473bc7c2399b494074aad15c9d61ffa55 Mon Sep 17 00:00:00 2001 From: Mauricio Villegas <5780272+mauvilsa@users.noreply.github.com> Date: Mon, 18 Aug 2025 07:05:38 +0200 Subject: [PATCH 12/14] Apply suggestions from code review --- jsonargparse/_signatures.py | 9 ++++----- jsonargparse_tests/test_signatures.py | 13 +++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/jsonargparse/_signatures.py b/jsonargparse/_signatures.py index 9547dcc7..59bfb836 100644 --- a/jsonargparse/_signatures.py +++ b/jsonargparse/_signatures.py @@ -354,13 +354,12 @@ def _add_signature_parameter( elif kind == kinds.KEYWORD_ONLY: is_required = default == inspect_empty # Required if no default is_non_positional = True # Must use --flag style - elif kind in {kinds.VAR_POSITIONAL, kinds.VAR_KEYWORD}: - # These parameter types don't translate well to CLI arguments - return # Skip entirely - else: - # Fallback for programmatically created parameters without kind + elif kind is None: + # programmatically created parameters without kind is_required = default == inspect_empty # Required if no default is_non_positional = False # Can be positional (preserve old behavior) + else: + raise RuntimeError(f"The code should never reach here: typehint={typehint}") # pragma: no cover src = get_parameter_origins(param.component, param.parent) skip_message = f'Skipping parameter "{name}" from "{src}" because of: ' if not fail_untyped and annotation == inspect_empty: diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index a87c1125..db113fec 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -699,16 +699,17 @@ def test_add_function_param_conflict(parser): ctx.match("Unable to add parameter 'cfg' from") -def func_positional_and_keyword(a: int, /, b: int, *, c: int, d: int = 1): +def func_positional_and_keyword_only(a: int, /, b: int, *, c: int, d: int = 1): pass -def test_add_function_positional_and_keyword_parameters(parser): +def test_add_function_positional_and_keyword_only_parameters(parser): parser.add_function_arguments(func_positional_and_keyword, as_positional=True) # Test that we can parse with both parameters cfg = parser.parse_args(["1", "2", "--c=3", "--d=4"]) - assert cfg.a == 1 - assert cfg.b == 2 - assert cfg.c == 3 - assert cfg.d == 4 + assert cfg == Namespace(a=1, b=2, c=3, d=4) + with pytest.raises(ArgumentError, match="Unrecognized arguments: --b=2"): + parser.parse_args(["1", "--b=2", "--c=3", "--d=4"]) + with pytest.raises(ArgumentError, match='Key "c" is required'): + parser.parse_args(["1", "2", "--d=4"]) From 17068dd4a50e2fe2071b750f709ecacf577463a3 Mon Sep 17 00:00:00 2001 From: Mauricio Villegas <5780272+mauvilsa@users.noreply.github.com> Date: Mon, 18 Aug 2025 07:07:49 +0200 Subject: [PATCH 13/14] Update jsonargparse_tests/test_signatures.py --- jsonargparse_tests/test_signatures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index db113fec..c936ce05 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -704,7 +704,7 @@ def func_positional_and_keyword_only(a: int, /, b: int, *, c: int, d: int = 1): def test_add_function_positional_and_keyword_only_parameters(parser): - parser.add_function_arguments(func_positional_and_keyword, as_positional=True) + parser.add_function_arguments(func_positional_and_keyword_only, as_positional=True) # Test that we can parse with both parameters cfg = parser.parse_args(["1", "2", "--c=3", "--d=4"]) From f2b2dc7493ff5249cc1baf799b36a748e1ef7aa1 Mon Sep 17 00:00:00 2001 From: Mauricio Villegas <5780272+mauvilsa@users.noreply.github.com> Date: Mon, 18 Aug 2025 07:08:49 +0200 Subject: [PATCH 14/14] Update jsonargparse/_signatures.py --- jsonargparse/_signatures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jsonargparse/_signatures.py b/jsonargparse/_signatures.py index 59bfb836..2ab26c57 100644 --- a/jsonargparse/_signatures.py +++ b/jsonargparse/_signatures.py @@ -359,7 +359,7 @@ def _add_signature_parameter( is_required = default == inspect_empty # Required if no default is_non_positional = False # Can be positional (preserve old behavior) else: - raise RuntimeError(f"The code should never reach here: typehint={typehint}") # pragma: no cover + raise RuntimeError(f"The code should never reach here: kind={kind}") # pragma: no cover src = get_parameter_origins(param.component, param.parent) skip_message = f'Skipping parameter "{name}" from "{src}" because of: ' if not fail_untyped and annotation == inspect_empty: