Repository: omni-us/jsonargparse
Pull request #645 (2024-12-17 06:58:51)

Fix help for Protocol types not working correctly
## What does this PR do?

Fixes https://github.com/omni-us/jsonargparse/issues/636#issuecomment-2541813386

## Before submitting

- [x] Did you read the [contributing guideline](https://github.com/omni-us/jsonargparse/blob/main/CONTRIBUTING.rst)?
- [n/a] Did you update **the documentation**? (readme and public docstrings)
- [x] Did you write **unit tests** such that there is 100% coverage on related code? (required for bug fixes and new features)
- [x] Did you verify that new and existing **tests pass locally**?
- [x] Did you make sure that all changes preserve **backward compatibility**?
- [x] Did you update **the CHANGELOG**? (not for typos, docs, test updates, or minor internal changes/refactors)


sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=645) **Quality Gate passed**  
Issues  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=645&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=645&issueStatuses=ACCEPTED)

Measures  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=omni-us_jsonargparse&pullRequest=645&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=645&metric=new_coverage&view=list)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=645&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=645)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/omni-us/jsonargparse/pull/645?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 100.00%. Comparing base [(`cd090e3`)](https://app.codecov.io/gh/omni-us/jsonargparse/commit/cd090e336262a6ceb374ebfcdde8f607a0cfdc2e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) to head [(`081d540`)](https://app.codecov.io/gh/omni-us/jsonargparse/commit/081d540b18d859afab35673846d4f7d76cff1f66?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us).

:white_check_mark: All tests successful. No failed tests found.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff            @@
##              main      #645   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines         6495      6499    +4     
=========================================
+ Hits          6495      6499    +4     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/omni-us/jsonargparse/pull/645?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us).

sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=645) **Quality Gate passed**  
Issues  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=645&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=645&issueStatuses=ACCEPTED)

Measures  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=omni-us_jsonargparse&pullRequest=645&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=645&metric=new_coverage&view=list)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=645&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=645)



Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 4ebc193..ff1ac63 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -12,6 +12,15 @@ The semantic versioning only considers the public API as described in
 paths are considered internals and can change in minor and patch releases.
 
 
+v4.35.1 (2024-12-??)
+--------------------
+
+Fixed
+^^^^^
+- Help for ``Protocol`` types not working correctly (`#645
+  <https://github.com/omni-us/jsonargparse/pull/645>`__).
+
+
 v4.35.0 (2024-12-16)
 --------------------
 
diff --git a/jsonargparse/_actions.py b/jsonargparse/_actions.py
index a3a55ed..47061fa 100644
--- a/jsonargparse/_actions.py
+++ b/jsonargparse/_actions.py
@@ -350,18 +350,31 @@ class _ActionHelpClassPath(Action):
             super().__init__(**kwargs)
 
     def update_init_kwargs(self, kwargs):
-        from ._typehints import get_optional_arg, get_subclass_names, get_unaliased_type
+        from ._typehints import (
+            get_optional_arg,
+            get_subclass_names,
+            get_subclass_types,
+            get_unaliased_type,
+            is_protocol,
+        )
 
         typehint = get_unaliased_type(get_optional_arg(self._typehint))
         if get_typehint_origin(typehint) is not Union:
             assert "nargs" not in kwargs
             kwargs["nargs"] = "?"
         self._basename = iter_to_set_str(get_subclass_names(self._typehint, callable_return=True))
+        self._baseclasses = get_subclass_types(typehint, callable_return=True)
+        assert self._baseclasses
+
+        self._kind = "subclass of"
+        if any(is_protocol(b) for b in self._baseclasses):
+            self._kind = "subclass or implementer of protocol"
+
         kwargs.update(
             {
                 "metavar": "CLASS_PATH_OR_NAME",
                 "default": SUPPRESS,
-                "help": f"Show the help for the given subclass of {self._basename} and exit.",
+                "help": f"Show the help for the given {self._kind} {self._basename} and exit.",
             }
         )
 
@@ -375,23 +388,22 @@ class _ActionHelpClassPath(Action):
         from ._typehints import (
             ActionTypeHint,
             get_optional_arg,
-            get_subclass_types,
             get_unaliased_type,
+            implements_protocol,
             resolve_class_path_by_name,
         )
 
         parser, _, value, option_string = call_args
         try:
             typehint = get_unaliased_type(get_optional_arg(self._typehint))
-            baseclasses = get_subclass_types(typehint, callable_return=True)
             if self.nargs == "?" and value is None:
                 val_class = typehint
             else:
                 val_class = import_object(resolve_class_path_by_name(typehint, value))
         except Exception as ex:
             raise TypeError(f"{option_string}: {ex}") from ex
-        if not any(is_subclass(val_class, b) for b in baseclasses):
-            raise TypeError(f'{option_string}: Class "{value}" is not a subclass of {self._basename}')
+        if not any(is_subclass(val_class, b) or implements_protocol(val_class, b) for b in self._baseclasses):
+            raise TypeError(f'{option_string}: Class "{value}" is not a {self._kind} {self._basename}')
         dest = re.sub("\\.help$", "", self.dest)
         subparser = type(parser)(description=f"Help for {option_string}={get_import_path(val_class)}")
         if ActionTypeHint.is_callable_typehint(typehint) and hasattr(typehint, "__args__"):
diff --git a/jsonargparse/_typehints.py b/jsonargparse/_typehints.py
index 4ac35b3..986d1d0 100644
--- a/jsonargparse/_typehints.py
+++ b/jsonargparse/_typehints.py
@@ -1103,7 +1103,7 @@ def implements_protocol(value, protocol) -> bool:
     from jsonargparse._parameter_resolvers import get_signature_parameters
     from jsonargparse._postponed_annotations import get_return_type
 
-    if not inspect.isclass(value) or value is object:
+    if not inspect.isclass(value) or value is object or not is_protocol(protocol):
         return False
     members = 0
     for name, _ in inspect.getmembers(protocol, predicate=inspect.isfunction):


Test patch:
diff --git a/jsonargparse_tests/test_subclasses.py b/jsonargparse_tests/test_subclasses.py
index a29e343..e5a8c87 100644
--- a/jsonargparse_tests/test_subclasses.py
+++ b/jsonargparse_tests/test_subclasses.py
@@ -1443,6 +1443,14 @@ class ImplementsInterface:
         return items
 
 
+class SubclassImplementsInterface(Interface):
+    def __init__(self, max_items: int):
+        self.max_items = max_items
+
+    def predict(self, items: List[float]) -> List[float]:
+        return items
+
+
 class NotImplementsInterface1:
     def predict(self, items: str) -> List[float]:
         return []
@@ -1462,6 +1470,7 @@ class NotImplementsInterface3:
     "expected, value",
     [
         (True, ImplementsInterface),
+        (True, SubclassImplementsInterface),
         (False, ImplementsInterface(1)),
         (False, NotImplementsInterface1),
         (False, NotImplementsInterface2),
@@ -1488,7 +1497,6 @@ def test_is_instance_or_supports_protocol(expected, value):
 
 def test_parse_implements_protocol(parser):
     parser.add_argument("--cls", type=Interface)
-    assert "known subclasses:" not in get_parser_help(parser)
     cfg = parser.parse_args([f"--cls={__name__}.ImplementsInterface", "--cls.batch_size=5"])
     assert cfg.cls.class_path == f"{__name__}.ImplementsInterface"
     assert cfg.cls.init_args == Namespace(batch_size=5)
@@ -1496,6 +1504,15 @@ def test_parse_implements_protocol(parser):
     assert isinstance(init.cls, ImplementsInterface)
     assert init.cls.batch_size == 5
     assert init.cls.predict([1.0, 2.0]) == [1.0, 2.0]
+
+    help_str = get_parser_help(parser)
+    assert "known subclasses:" in help_str
+    assert f"{__name__}.SubclassImplementsInterface" in help_str
+    help_str = get_parse_args_stdout(parser, ["--cls.help=SubclassImplementsInterface"])
+    assert "--cls.max_items" in help_str
+    with pytest.raises(ArgumentError, match="not a subclass or implementer of protocol"):
+        parser.parse_args([f"--cls.help={__name__}.NotImplementsInterface1"])
+
     with pytest.raises(ArgumentError, match="is a protocol"):
         parser.parse_args([f"--cls={__name__}.Interface"])
     with pytest.raises(ArgumentError, match="does not implement protocol"):
@@ -1551,13 +1568,17 @@ def test_implements_callable_protocol(expected, value):
 
 def test_parse_implements_callable_protocol(parser):
     parser.add_argument("--cls", type=CallableInterface)
-    assert "known subclasses:" not in get_parser_help(parser)
     cfg = parser.parse_args([f"--cls={__name__}.ImplementsCallableInterface", "--cls.batch_size=7"])
     assert cfg.cls.class_path == f"{__name__}.ImplementsCallableInterface"
     assert cfg.cls.init_args == Namespace(batch_size=7)
     init = parser.instantiate_classes(cfg)
     assert isinstance(init.cls, ImplementsCallableInterface)
     assert init.cls([1.0, 2.0]) == [1.0, 2.0]
+
+    assert "known subclasses:" not in get_parser_help(parser)
+    help_str = get_parse_args_stdout(parser, [f"--cls.help={__name__}.ImplementsCallableInterface"])
+    assert "--cls.batch_size" in help_str
+
     with pytest.raises(ArgumentError, match="is a protocol"):
         parser.parse_args([f"--cls={__name__}.CallableInterface"])
     with pytest.raises(ArgumentError, match="does not implement protocol"):