Fix/callable protocols 2

MiguelMonteiro commented 1 month ago

What does this PR do?

Fixes callable protocol inheritance by allowing a list of private methods to be considered when matching signatures for inheritance of subclasses. The set of allowed_dunder_methods currently only contains the __call__ method.

Fixes #595

Before submitting

[x] Did you read the contributing guideline?
[ ] Did you update the documentation? (readme and public docstrings)
[ ] 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)

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (442f9bf) to head (bd03440). Report is 3 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #599 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 22 22 Lines 6491 6493 +2 ========================================= + Hits 6491 6493 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

MiguelMonteiro commented 1 month ago

Please add a unit test for a protocol with __call__.

I added some tests for callable protocols. One question that came up is whether functions should be checked against the protocol. Currently that test is failing, here is the example:

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

def implements_callable_interface2(items: List[float]) -> List[float]:
    return items

The result is:

 implements_protocol(ImplementsCallableInterface1, CallableInterface) = True
implements_protocol(implements_callable_interface2, CallableInterface) = False

MiguelMonteiro commented 3 weeks ago

Thanks for the feedback, sorry it's been really busy at work, I will try to get to this next week

MiguelMonteiro commented 3 weeks ago

I made the necessary fixes, currently only one subtest is failing but I don't fully understand why.

@pytest.mark.parametrize(
    "expected, value",
    [
        (True, ImplementsCallableInterface1),
        (False, ImplementsCallableInterface1(1)),
        (
            False,
            implements_callable_interface2,
        ),  # TODO: switch to True once functions that implement protocol are checked correctly
        (False, NotImplementsCallableInterface1),
        (False, NotImplementsCallableInterface2),
        (False, NotImplementsCallableInterface3),
        (False, not_implements_callable_interface4),
        (False, object),
    ],
)
def test_implements_callable_protocol(expected, value):
    assert implements_protocol(value, CallableInterface) is expected

The last subtest (False, object) raises the following error instead of returning False

FAILED jsonargparse_tests/test_subclasses.py::test_implements_callable_protocol[False-object] - ValueError: Invalid or unsupported input: class=<class 'object'>, method_or_property=__call__

It does not happen in the normal (not callable) protocol tests.