Repository: omni-us/jsonargparse

Issue:
Callable protocols inheritance broken
<!-- If you like this project, please ⭐ star it https://github.com/omni-us/jsonargparse/ -->

## 🐛 Bug report

Callable protocols (implement `__call__`) seems to be parser incorrectly in comparison to "normal" protocols.

### To reproduce

Example that works
```
from dataclasses import dataclass
from typing import Protocol

from jsonargparse import ArgumentParser


class BaseGood(Protocol):
    def predict(self, a: int) -> int: ...


@dataclass
class SpecializedGood(BaseGood):
    number: int = 5

    def predict(self, a: int) -> int:
        return a + 5


def main() -> None:
    parser = ArgumentParser(description="CLI for training models", exit_on_error=False)
    parser.add_argument("--cfg", type=BaseGood)
    args = parser.parse_args(["--cfg=SpecializedGood", "--cfg.number=5"])
    print(args)
    
if __name__ == "__main__":
    main()
```

Example that does not work:
```
class BaseBad(Protocol):
    def __call__(self, a: int) -> int: ...


@dataclass
class SpecializedBad(BaseBad):
    number: int = 5

    def __call__(self, a: int) -> int:
        return a + 5


def main() -> None:
    parser = ArgumentParser(description="CLI for training models", exit_on_error=False)
    parser.add_argument("--cfg", type=BaseBad)
    args = parser.parse_args(["--cfg=SpecializedBad", "--cfg.number=5"])
    print(args)
    
if __name__ == "__main__":
    main()
```

The second example raises the following error:
```
Import path SpecializedBad does not correspond to a subclass of BaseBad
```
Seems like the parser does not match the inherited protocol signature to the base if the protocol methods are private (?).


### Expected behavior

I would expect the second example to work like the first.

### Environment

<!-- Fill in the list below. -->

- jsonargparse version: 4.33.1
- Python version: 3.10
- How jsonargparse was installed: pip
- OS: MacOS

Hints from issue comments:
@mauvilsa: Thank you for reporting!

Indeed the intention was to not consider private methods when checking whether a protocol is implemented. This is here https://github.com/omni-us/jsonargparse/blob/2825a7357908e324b48e9512b721158c028310a3/jsonargparse/_typehints.py#L1096-L1097

Though, it was not intended to exclude `__call__`, just it wasn't noticed. Would be an easy fix. Just change the condition to
```python
if name.startswith("_") and name != "__call__":
```
Do you want to contribute the fix?
@MiguelMonteiro: Thanks, yes sure I will fix it@MiguelMonteiro: what is the reason for excluding private methods though? @mauvilsa: > what is the reason for excluding private methods though?

The purpose of a `Protocol` is to define the **public** interface that a family of classes should adhere to. Even though it might be possible to add a private method to a protocol, that would be discouraged and considered a bad practice. I would not add support for things that are known to be bad practice.

But note that dunder methods, such as `__call__` are not private methods. Which is why it does make sense to consider for `Protocols`. And it is not just `__call__`. There are other dunder methods that would also make sense for protocols. Though there are also dunder methods that don't make sense. For example, `__len__` might make sense, whereas `__del__` wouldn't make sense. A question could be, do we add now support for more than `__call__`? I would prefer to not simply allow any dunder method, since for the cases that doesn't make sense, better to not support it. Could also be that we only add support for `__call__` now and only extend when someone requests it, which would mean that an actual use case for it has been found.@MiguelMonteiro: I see your point, maybe we can have a list of allowed dunder methods which can be expanded when needed? 

However, I do think it might be out of the scope of the parser to try to enforce/disallow discouraged practices. Unless there is consistent rule about which dunder methods are allowed, it will be hard to predict the expected behavior of the parser. Is there any other downside to removing the check:
https://github.com/omni-us/jsonargparse/blob/2825a7357908e324b48e9512b721158c028310a3/jsonargparse/_typehints.py#L1096-L1097@mauvilsa: The good practices is a design decision, similar to the one for [lightning](https://github.com/Lightning-AI/pytorch-lightning/blob/master/.github/CONTRIBUTING.md#force-user-decisions-to-best-practices). Also in practical terms, (not specific to this case) allowing unexpected uses, can mean that people implement and rely on behaviors which were not really intended, and add to the maintainance cost of the library. For sure allowing private methods will not happen. Dunder methods is something to analyze.@MiguelMonteiro: Ok, makes sense. I created a [PR](https://github.com/omni-us/jsonargparse/pull/599) which adds a set of exceptions for dunder methods and added `__call__` to said set. This can be expanded in the future if needed.

Solution patch:
diff --git a/.bumpversion.cfg b/.bumpversion.cfg
index 2c06678d..619564f8 100644
--- a/.bumpversion.cfg
+++ b/.bumpversion.cfg
@@ -9,5 +9,3 @@ serialize =
 	{major}.{minor}.{patch}
 
 [bumpversion:file:jsonargparse/__init__.py]
-
-[bumpversion:file:.sonarcloud.properties]
diff --git a/.circleci/config.yml b/.circleci/config.yml
deleted file mode 100644
index b29b175f..00000000
--- a/.circleci/config.yml
+++ /dev/null
@@ -1,186 +0,0 @@
-version: 2.1
-
-jobs:
-  build:
-    docker:
-      - image: cimg/python:3.10
-    steps:
-      - checkout
-      - run:
-          name: Build wheel package
-          command: |
-            pip3 install -U build
-            python3 -m build
-      - persist_to_workspace:
-          root: .
-          paths:
-            - ./dist/*.whl
-            - ./dist/*.tar.gz
-  test-py38: &test-py38
-    docker:
-      - image: cimg/python:3.8
-    steps:
-      - checkout
-      - run:
-          name: Test without optional dependencies
-          command: |
-            py=$(python3 --version | sed -r 's|.* 3\.([0-9]+)\..*|3.\1|')
-            pip3 install .[coverage]
-            pip3 uninstall -y argcomplete
-            pytest --cov --cov-report=term
-            mv .coverage coverage_py$py
-      - run:
-          name: Test with all optional dependencies
-          command: |
-            py=$(python3 --version | sed -r 's|.* 3\.([0-9]+)\..*|3.\1|')
-            pip3 install .[test,all]
-            pytest --cov --cov-report=term
-            mv .coverage coverage_py${py}_all
-      - run:
-          name: Test without future annotations and all optional dependencies
-          command: |
-            py=$(python3 --version | sed -r 's|.* 3\.([0-9]+)\..*|3.\1|')
-            sed -i '/^from __future__ import annotations$/d' jsonargparse_tests/test_*.py
-            pytest --cov --cov-report=term
-            mv .coverage coverage_py${py}_types
-      - persist_to_workspace:
-          root: .
-          paths:
-            - ./coverage_py*
-  test-py313:
-    <<: *test-py38
-    docker:
-      - image: cimg/python:3.13
-  test-py312:
-    <<: *test-py38
-    docker:
-      - image: cimg/python:3.12
-  test-py311:
-    <<: *test-py38
-    docker:
-      - image: cimg/python:3.11
-  test-py310:
-    <<: *test-py38
-    docker:
-      - image: cimg/python:3.10
-  test-py39:
-    <<: *test-py38
-    docker:
-      - image: cimg/python:3.9
-  test-py310-pydantic-v1:
-    docker:
-      - image: cimg/python:3.10
-    steps:
-      - checkout
-      - run:
-          name: Test with pydantic<2
-          command: |
-            py=$(python3 --version | sed -r 's|.* 3\.([0-9]+)\..*|3.\1|')
-            pip3 install .[coverage]
-            pip3 install "pydantic<2"
-            pytest --cov --cov-report=term jsonargparse_tests/test_dataclass_like.py
-            mv .coverage coverage_py${py}_pydantic1
-      - run:
-          name: Test with pydantic>=2 importing from pydantic.v1
-          command: |
-            py=$(python3 --version | sed -r 's|.* 3\.([0-9]+)\..*|3.\1|')
-            sed -i "s|import pydantic|import pydantic.v1 as pydantic|" jsonargparse_tests/test_dataclass_like.py
-            sed -i "s|^annotated = .*|annotated = False|" jsonargparse_tests/test_dataclass_like.py
-            sed -i "s|test_pydantic_types|_test_pydantic_types|" jsonargparse_tests/test_dataclass_like.py
-            pip3 install "pydantic>=2"
-            pytest --cov --cov-report=term jsonargparse_tests/test_dataclass_like.py
-            mv .coverage coverage_py${py}_pydantic2
-      - persist_to_workspace:
-          root: .
-          paths:
-            - ./coverage_py*
-  codecov:
-    docker:
-      - image: cimg/python:3.10
-    steps:
-      - checkout
-      - attach_workspace:
-          at: .
-      - run:
-          name: Upload code coverage
-          command: |
-            pip3 install -U coverage codecov-cli
-            coverage combine coverage_py*
-            coverage xml
-            codecovcli upload-process \
-              --fail-on-error \
-              --token $CODECOV_TOKEN_JSONARGPARSE \
-              --file coverage.xml
-  test-py310-installed:
-    docker:
-      - image: cimg/python:3.10
-    steps:
-      - attach_workspace:
-          at: .
-      - run:
-          name: Test without optional dependencies
-          command: |
-            pip3 install $(ls ./dist/*.whl)[test-no-urls]
-            pip3 uninstall -y argcomplete
-            python3 -m jsonargparse_tests
-      - run:
-          name: Test with all optional dependencies
-          command: |
-            pip3 install $(ls ./dist/*.whl)[test,all]
-            python3 -m jsonargparse_tests
-  publish-pypi:
-    docker:
-      - image: cimg/python:3.10
-    steps:
-      - attach_workspace:
-          at: .
-      - run:
-          name: Publish Release on PyPI
-          command: |
-            pip3 install -U twine
-            twine upload --username __token__ --password "${PYPI_TOKEN}" ./dist/*.whl ./dist/*.tar.gz
-
-workflows:
-  version: 2
-  build-test-publish:
-    jobs:
-      - build: &buildreq
-          filters: &tagfilter
-            tags:
-              only: /^v\d+\.\d+\.\d+.*$/
-      - test-py38:
-          <<: *buildreq
-      - test-py313:
-          <<: *buildreq
-      - test-py312:
-          <<: *buildreq
-      - test-py311:
-          <<: *buildreq
-      - test-py310:
-          <<: *buildreq
-      - test-py39:
-          <<: *buildreq
-      - test-py310-pydantic-v1:
-          <<: *buildreq
-      - codecov:
-          requires:
-            - test-py313
-            - test-py312
-            - test-py311
-            - test-py310
-            - test-py39
-            - test-py38
-            - test-py310-pydantic-v1
-      - test-py310-installed:
-          <<: *buildreq
-          requires:
-            - build
-      - publish-pypi:
-          filters:
-            branches:
-              ignore: /.*/
-            <<: *tagfilter
-          context: pypi-upload-context
-          requires:
-            - codecov
-            - test-py310-installed
diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yaml
similarity index 100%
rename from .github/workflows/codeql.yml
rename to .github/workflows/codeql.yaml
diff --git a/.github/workflows/coverage.yaml b/.github/workflows/coverage.yaml
new file mode 100644
index 00000000..f5ae4689
--- /dev/null
+++ b/.github/workflows/coverage.yaml
@@ -0,0 +1,148 @@
+name: coverage
+
+on:
+  push:
+    branches: [ "main" ]
+    tags:
+      - 'v*'
+  pull_request:
+    branches: [ "main" ]
+
+jobs:
+
+  linux:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python: [3.8, 3.9, "3.10", 3.11, 3.12, 3.13]
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: ${{ matrix.python }}
+          cache: pip
+      - name: without optional dependencies
+        run: |
+          pip install .[coverage]
+          pip uninstall -y argcomplete
+          pytest --cov --cov-report=term --cov-report=xml --junit-xml=junit.xml
+          mv coverage.xml coverage_py${{ matrix.python }}_bare.xml
+          mv junit.xml junit_py${{ matrix.python }}_bare.xml
+      - name: with all optional dependencies
+        run: |
+          pip install .[test,all]
+          pytest --cov --cov-report=term --cov-report=xml --junit-xml=junit.xml
+          mv coverage.xml coverage_py${{ matrix.python }}_all.xml
+          mv junit.xml junit_py${{ matrix.python }}_all.xml
+      - name: without future annotations
+        run: |
+          sed -i '/^from __future__ import annotations$/d' jsonargparse_tests/test_*.py
+          pytest --cov --cov-report=term --cov-report=xml --junit-xml=junit.xml
+          mv coverage.xml coverage_py${{ matrix.python }}_types.xml
+          mv junit.xml junit_py${{ matrix.python }}_types.xml
+      - uses: actions/upload-artifact@v4
+        with:
+          name: coverage_py${{ matrix.python }}
+          path: ./coverage_py*
+      - uses: actions/upload-artifact@v4
+        with:
+          name: junit_py${{ matrix.python }}
+          path: ./junit_py*
+
+  linux-pydantic-v1:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.10"
+          cache: pip
+      - name: With pydantic<2
+        run: |
+          pip install .[coverage]
+          pip install "pydantic<2"
+          pytest --cov --cov-report=term --cov-report=xml --junit-xml=junit.xml jsonargparse_tests/test_dataclass_like.py
+          mv coverage.xml coverage_pydantic1.xml
+          mv junit.xml junit_pydantic1.xml
+      - name: with pydantic>=2
+        run: |
+          sed -i "s|import pydantic|import pydantic.v1 as pydantic|" jsonargparse_tests/test_dataclass_like.py
+          sed -i "s|^annotated = .*|annotated = False|" jsonargparse_tests/test_dataclass_like.py
+          sed -i "s|test_pydantic_types|_test_pydantic_types|" jsonargparse_tests/test_dataclass_like.py
+          pip install "pydantic>=2"
+          pytest --cov --cov-report=term --cov-report=xml --junit-xml=junit.xml jsonargparse_tests/test_dataclass_like.py
+          mv coverage.xml coverage_pydantic2.xml
+          mv junit.xml junit_pydantic2.xml
+      - uses: actions/upload-artifact@v4
+        with:
+          name: coverage_pydantic
+          path: ./coverage_py*
+      - uses: actions/upload-artifact@v4
+        with:
+          name: junit_pydantic
+          path: ./junit_py*
+
+  codecov:
+    runs-on: ubuntu-latest
+    needs: [linux, linux-pydantic-v1]
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/download-artifact@v4
+        with:
+          merge-multiple: true
+      - uses: codecov/codecov-action@v4
+        with:
+          fail_ci_if_error: true
+          files: ./coverage_*.xml
+          token: ${{ secrets.CODECOV_TOKEN }}
+      - uses: codecov/test-results-action@v1
+        with:
+          fail_ci_if_error: true
+          files: ./junit_*.xml
+          token: ${{ secrets.CODECOV_TOKEN }}
+
+  sonarcloud:
+    runs-on: ubuntu-latest
+    needs: [linux, linux-pydantic-v1]
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0  # Shallow clone disabled for a better relevancy of analysis
+      - uses: actions/download-artifact@v4
+        with:
+          merge-multiple: true
+      - uses: SonarSource/sonarcloud-github-action@master
+        with:
+          args: >
+            -Dsonar.organization=omni-us
+            -Dsonar.projectKey=omni-us_jsonargparse
+            -Dsonar.sources=jsonargparse
+            -Dsonar.exclusions=sphinx/**
+            -Dsonar.tests=jsonargparse_tests
+            -Dsonar.python.coverage.reportPaths=coverage_*.xml
+            -Dsonar.python.version=3.8,3.9,3.10,3.11,3.12,3.13
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}  # Needed to get PR information, if any
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+
+  publish-pypi:
+    if: startsWith(github.ref, 'refs/tags/v')
+    runs-on: ubuntu-latest
+    needs: [linux, linux-pydantic-v1]
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.10"
+      - name: Build package
+        run: |
+          pip install -U build
+          python -m build
+          cd dist
+      - name: Publish to PyPI
+        env:
+          TWINE_USERNAME: __token__
+          TWINE_PASSWORD: ${{ secrets.PYPI_TOKEN }}
+        run: |
+          pip install -U twine
+          twine upload ./dist/*.whl ./dist/*.tar.gz
diff --git a/.github/workflows/manual.yml b/.github/workflows/manual.yaml
similarity index 100%
rename from .github/workflows/manual.yml
rename to .github/workflows/manual.yaml
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yaml
similarity index 72%
rename from .github/workflows/tests.yml
rename to .github/workflows/tests.yaml
index e3d506fe..31dd7ada 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yaml
@@ -1,4 +1,5 @@
 name: tests
+
 on:
   push:
     branches: [ "main" ]
@@ -7,7 +8,7 @@ on:
 
 jobs:
 
-  unittest-windows:
+  windows:
     runs-on: windows-2019
     strategy:
       fail-fast: false
@@ -19,11 +20,10 @@ jobs:
         with:
           python-version: ${{ matrix.python }}
           cache: pip
-          allow-prereleases: true
       - run: pip install tox
       - run: tox -e py-all-extras
 
-  unittest-macos:
+  macos:
     runs-on: macOS-15
     strategy:
       fail-fast: false
@@ -38,7 +38,7 @@ jobs:
       - run: pip install tox
       - run: tox -e py-all-extras
 
-  unittest-omegaconf:
+  omegaconf:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
@@ -49,6 +49,29 @@ jobs:
       - run: pip install tox
       - run: tox -e omegaconf
 
+  installed-package:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.10"
+          cache: pip
+      - name: Build package
+        run: |
+          pip install -U build
+          python -m build
+      - name: Test without optional dependencies
+        run: |
+          cd dist
+          pip install $(ls *.whl)[test-no-urls]
+          python -m jsonargparse_tests
+      - name: Test with all optional dependencies
+        run: |
+          cd dist
+          pip install $(ls *.whl)[test,all]
+          python -m jsonargparse_tests
+
   doctest:
     runs-on: ubuntu-latest
     steps:
diff --git a/.sonarcloud.properties b/.sonarcloud.properties
deleted file mode 100644
index 12f38fa3..00000000
--- a/.sonarcloud.properties
+++ /dev/null
@@ -1,3 +0,0 @@
-sonar.sources=jsonargparse
-sonar.projectVersion=4.33.2
-sonar.python.version=3.8, 3.9, 3.10, 3.11, 3.12, 3.13
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 84bb8c6b..d0d76494 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -33,6 +33,8 @@ Fixed
   <https://github.com/omni-us/jsonargparse/pull/608>`__).
 - Failure when resolving forward references from dataclass parameter types
   (`#611 <https://github.com/omni-us/jsonargparse/pull/611>`__).
+- Fix callable protocol inheritance.
+  (`#599 <https://github.com/omni-us/jsonargparse/pull/599>`__).
 
 Changed
 ^^^^^^^
diff --git a/README.rst b/README.rst
index 68a41224..96da2671 100644
--- a/README.rst
+++ b/README.rst
@@ -1,7 +1,7 @@
 .. image:: https://readthedocs.org/projects/jsonargparse/badge/?version=stable
     :target: https://readthedocs.org/projects/jsonargparse/
-.. image:: https://github.com/omni-us/jsonargparse/actions/workflows/tests.yml/badge.svg
-    :target: https://github.com/omni-us/jsonargparse/actions/workflows/tests.yml
+.. image:: https://github.com/omni-us/jsonargparse/actions/workflows/tests.yaml/badge.svg
+    :target: https://github.com/omni-us/jsonargparse/actions/workflows/tests.yaml
 .. image:: https://codecov.io/gh/omni-us/jsonargparse/branch/main/graph/badge.svg
     :target: https://codecov.io/gh/omni-us/jsonargparse
 .. image:: https://sonarcloud.io/api/project_badges/measure?project=omni-us_jsonargparse&metric=alert_status
diff --git a/jsonargparse/_typehints.py b/jsonargparse/_typehints.py
index 4ebaba10..c8f5afb4 100644
--- a/jsonargparse/_typehints.py
+++ b/jsonargparse/_typehints.py
@@ -1087,15 +1087,32 @@ def adapt_typehints(
     return val
 
 
+protocol_irrelevant_dunder_methods = {
+    "__init__",
+    "__new__",
+    "__del__",
+    "__getattr__",
+    "__getattribute__",
+    "__setattr__",
+    "__delattr__",
+    "__reduce__",
+    "__reduce_ex__",
+    "__getstate__",
+    "__setstate__",
+    "__subclasshook__",
+}
+
+
 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):
+    if not inspect.isclass(value) or value is object:
         return False
     members = 0
     for name, _ in inspect.getmembers(protocol, predicate=inspect.isfunction):
-        if name.startswith("_"):
+        is_dunder = name.startswith("__") and name.endswith("__")
+        if (not is_dunder and name.startswith("_")) or (is_dunder and name in protocol_irrelevant_dunder_methods):
             continue
         if not hasattr(value, name):
             return False
diff --git a/pyproject.toml b/pyproject.toml
index cd68641c..f184bfcb 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -152,6 +152,7 @@ testpaths = ["jsonargparse_tests"]
 
 
 [tool.coverage.run]
+relative_files = true
 source = ["jsonargparse"]
 
 


Test patch:
diff --git a/jsonargparse_tests/test_subclasses.py b/jsonargparse_tests/test_subclasses.py
index e433fc2a..6dc34c36 100644
--- a/jsonargparse_tests/test_subclasses.py
+++ b/jsonargparse_tests/test_subclasses.py
@@ -1500,6 +1500,51 @@ def test_parse_implements_protocol(parser):
     ctx.match("Not a valid subclass of Interface")
 
 
+# callable protocol tests
+
+
+class CallableInterface(Protocol):
+    def __call__(self, items: List[float]) -> List[float]: ...
+
+
+class ImplementsCallableInterface1:
+    def __init__(self, batch_size: int):
+        self.batch_size = batch_size
+
+    def __call__(self, items: List[float]) -> List[float]:
+        return items
+
+
+class NotImplementsCallableInterface1:
+    def __call__(self, items: str) -> List[float]:
+        return []
+
+
+class NotImplementsCallableInterface2:
+    def __call__(self, items: List[float], extra: int) -> List[float]:
+        return items
+
+
+class NotImplementsCallableInterface3:
+    def __call__(self, items: List[float]) -> None:
+        return
+
+
+@pytest.mark.parametrize(
+    "expected, value",
+    [
+        (True, ImplementsCallableInterface1),
+        (False, ImplementsCallableInterface1(1)),
+        (False, NotImplementsCallableInterface1),
+        (False, NotImplementsCallableInterface2),
+        (False, NotImplementsCallableInterface3),
+        (False, object),
+    ],
+)
+def test_implements_callable_protocol(expected, value):
+    assert implements_protocol(value, CallableInterface) is expected
+
+
 # parameter skip tests