test(sigcheck): check function signature parity across backends (#10008)

Opening this in favor of #9383 -- that PR also included all of the
breaking changes to unify function signatures and it was too much at
once. This PR adds only the signature checking mechanism, plus the
requisite xfails to lay out which inconsistencies are currently in Ibis.

## Motivation

We want to ensure that, for a given backend, that the argument names,
plus usage of positional, positional-only, keyword, and keyword-only
arguments match, so that there is API consistency when moving between
backends.

I've grabbed a few small parts of some of the utilities in Scott
Sanderson's
`python-interface` project
(https://github.com/ssanderson/python-interface).

While the upstream is no longer maintained, the goal of that project
aligns quite well with some of the issues we face with maintaining
consistent interfaces across backends.

Note that while the upstream project focused on _runtime_ enforcement of
these signatures matching, here it is only run in the test suite.

## Rough procedure

Any method that doesn't match can be skipped entirely (this is useful
for things like `do_connect`, which cannot reasonably be assumed to
match) or individually (by specifying a `pytest.param` and marking the
failing backends).

Then we scrape across the common parent classes and add any methods that
are NOT currently specified in the pre-existing xfailed ones. It's a bit
of a nuisance, but it's done, and ideally the manual listing of the
inconsistent methods goes away as we unify things.


I've opted for not checking that type annotations match, because that
seems... unreasonable.

This would satisfy #9125 once all of the xfail markers are removed,
e.g., it checks that all keyword and positional arguments are
standardized.

Author: gforsyth

Diff for: ibis/backends/conftest.py

@@ -26,6 +26,7 @@
 from ibis.util import promote_tuple
 
 if TYPE_CHECKING:
+    from ibis.backends import BaseBackend
     from ibis.backends.tests.base import BackendTest
 
 
@@ -311,6 +312,14 @@ def pytest_runtest_call(item):
     if not backend:
         # Check item path to see if test is in backend-specific folder
         backend = set(_get_backend_names()).intersection(item.path.parts)
+    if not backend:
+        # Check if this is one of the uninstantiated backend class fixture
+        # used for signature checking
+        backend = [
+            backend.name
+            for key, backend in item.funcargs.items()
+            if key.endswith("backend_cls")
+        ]
 
     if not backend:
         return
@@ -390,6 +399,22 @@ def _filter_none_from_raises(kwargs):
             item.add_marker(pytest.mark.xfail(reason=reason, **kwargs))
 
 
+def _get_backend_cls(backend_str: str):
+    """Convert a backend string to the test class for the backend."""
+    backend_mod = importlib.import_module(f"ibis.backends.{backend_str}")
+    return backend_mod.Backend
+
+
+@pytest.fixture(params=_get_backends_to_test(), scope="session")
+def backend_cls(request) -> BaseBackend:
+    """Return the uninstantiated backend class, unconnected.
+
+    This is used for signature checking and nothing should be executed."""
+
+    cls = _get_backend_cls(request.param)
+    return cls
+
+
 @pytest.fixture(params=_get_backends_to_test(), scope="session")
 def backend(request, data_dir, tmp_path_factory, worker_id) -> BackendTest:
     """Return an instance of BackendTest, loaded with data."""

Diff for: ibis/backends/tests/signature/__init__.py

@@ -0,0 +1,129 @@
+from __future__ import annotations  # noqa: INP001
+
+from inspect import signature
+from typing import Any
+
+import pytest
+from pytest import param
+
+from ibis.backends.tests.signature.typecheck import compatible
+
+
+def a1(posarg: int): ...
+
+
+def b1(posarg: str): ...
+
+
+def a2(posarg: int, **kwargs: Any): ...
+def b2(posarg: str, **kwargs): ...
+
+
+def a3(posarg: int, other_kwarg: bool = True, **kwargs: Any): ...
+def b3(posarg: str, **kwargs: Any): ...
+
+
+def a4(posarg: int, other_kwarg=True, **kwargs: Any): ...
+def b4(posarg: str, **kwargs: Any): ...
+
+
+def a5(posarg: int, /): ...
+def b5(posarg2: str, /): ...
+
+
+@pytest.mark.parametrize(
+    "a, b, check_annotations",
+    [
+        param(
+            lambda posarg, *, kwarg1=None, kwarg2=None: ...,
+            lambda posarg, *, kwarg2=None, kwarg1=None: ...,
+            True,
+            id="swapped kwarg order",
+        ),
+        param(
+            lambda posarg, *, kwarg1=None, kwarg2=None, kwarg3=None: ...,
+            lambda posarg, *, kwarg2=None, kwarg1=None: ...,
+            True,
+            id="swapped kwarg order w/extra kwarg first",
+        ),
+        param(
+            lambda posarg, *, kwarg2=None, kwarg1=None: ...,
+            lambda posarg, *, kwarg1=None, kwarg2=None, kwarg3=None: ...,
+            True,
+            id="swapped kwarg order w/extra kwarg second",
+        ),
+        param(
+            a1,
+            b1,
+            False,
+            id="annotations diff types w/out anno check",
+        ),
+        param(
+            a2,
+            b3,
+            False,
+            id="annotations different but parity in annotations",
+        ),
+        param(
+            a3,
+            b3,
+            False,
+            id="annotations different but parity in annotations for matching kwargs",
+        ),
+        param(
+            a4,
+            b4,
+            False,
+            id="annotations different but parity in annotations for matching kwargs",
+        ),
+        param(
+            a2,
+            b2,
+            False,
+            id="annotations different, no anno check, but missing annotation",
+        ),
+    ],
+)
+def test_sigs_compatible(a, b, check_annotations):
+    sig_a, sig_b = signature(a), signature(b)
+    assert compatible(sig_a, sig_b, check_annotations=check_annotations)
+
+
+@pytest.mark.parametrize(
+    "a, b, check_annotations",
+    [
+        param(
+            lambda posarg, /, *, kwarg2=None, kwarg1=None: ...,
+            lambda posarg, *, kwarg1=None, kwarg2=None, kwarg3=None: ...,
+            True,
+            id="one positional only",
+        ),
+        param(
+            lambda posarg, *, kwarg1=None, kwarg2=None: ...,
+            lambda posarg, kwarg1=None, kwarg2=None: ...,
+            True,
+            id="not kwarg only",
+        ),
+        param(
+            a1,
+            b1,
+            True,
+            id="annotations diff types w/anno check",
+        ),
+        param(
+            a2,
+            b3,
+            True,
+            id="annotations different but parity in annotations",
+        ),
+        param(
+            a5,
+            b5,
+            False,
+            id="names different, but positional only",
+        ),
+    ],
+)
+def test_sigs_incompatible(a, b, check_annotations):
+    sig_a, sig_b = signature(a), signature(b)
+    assert not compatible(sig_a, sig_b, check_annotations=check_annotations)

Diff for: ibis/backends/tests/signature/tests/test_compatible.py

@@ -0,0 +1,132 @@
+"""The following was forked from the python-interface project:
+
+* Copyright (c) 2016-2021, Scott Sanderson
+
+Utilities for typed interfaces.
+"""
+
+# ruff: noqa: D205, D415, D400
+
+from __future__ import annotations
+
+from functools import partial
+from inspect import Parameter, Signature
+from itertools import starmap, takewhile, zip_longest
+
+
+def valfilter(f, d):
+    return {k: v for k, v in d.items() if f(v)}
+
+
+def dzip(left, right):
+    return {k: (left.get(k), right.get(k)) for k in left.keys() & right.keys()}
+
+
+def complement(f):
+    def not_f(*args, **kwargs):
+        return not f(*args, **kwargs)
+
+    return not_f
+
+
+def compatible(
+    impl_sig: Signature, iface_sig: Signature, check_annotations: bool = True
+) -> bool:
+    """Check whether ``impl_sig`` is compatible with ``iface_sig``.
+
+    Parameters
+    ----------
+    impl_sig
+        The signature of the implementation function.
+    iface_sig
+        The signature of the interface function.
+    check_annotations
+        Whether to also compare signature annotations (default) vs only parameter names.
+
+    In general, an implementation is compatible with an interface if any valid
+    way of passing parameters to the interface method is also valid for the
+    implementation.
+
+    Consequently, the following differences are allowed between the signature
+    of an implementation method and the signature of its interface definition:
+
+    1. An implementation may add new arguments to an interface iff:
+       a. All new arguments have default values.
+       b. All new arguments accepted positionally (i.e. all non-keyword-only
+          arguments) occur after any arguments declared by the interface.
+       c. Keyword-only arguments may be reordered by the implementation.
+
+    2. For type-annotated interfaces, type annotations my differ as follows:
+       a. Arguments to implementations of an interface may be annotated with
+          a **superclass** of the type specified by the interface.
+       b. The return type of an implementation may be annotated with a
+          **subclass** of the type specified by the interface.
+    """
+    # Unwrap to get the underlying inspect.Signature objects.
+    return all(
+        [
+            positionals_compatible(
+                takewhile(is_positional, impl_sig.parameters.values()),
+                takewhile(is_positional, iface_sig.parameters.values()),
+                check_annotations=check_annotations,
+            ),
+            keywords_compatible(
+                valfilter(complement(is_positional), impl_sig.parameters),
+                valfilter(complement(is_positional), iface_sig.parameters),
+                check_annotations=check_annotations,
+            ),
+        ]
+    )
+
+
+_POSITIONALS = frozenset([Parameter.POSITIONAL_ONLY, Parameter.POSITIONAL_OR_KEYWORD])
+
+
+def is_positional(arg):
+    return arg.kind in _POSITIONALS
+
+
+def has_default(arg):
+    """Does ``arg`` provide a default?."""
+    return arg.default is not Parameter.empty
+
+
+def params_compatible(impl, iface, check_annotations=True):
+    if impl is None:
+        return False
+
+    if iface is None:
+        return has_default(impl)
+
+    checks = (
+        impl.name == iface.name
+        and impl.kind == iface.kind
+        and has_default(impl) == has_default(iface)
+    )
+
+    if check_annotations:
+        checks = checks and annotations_compatible(impl, iface)
+
+    return checks
+
+
+def positionals_compatible(impl_positionals, iface_positionals, check_annotations=True):
+    params_compat = partial(params_compatible, check_annotations=check_annotations)
+    return all(
+        starmap(
+            params_compat,
+            zip_longest(impl_positionals, iface_positionals),
+        )
+    )
+
+
+def keywords_compatible(impl_keywords, iface_keywords, check_annotations=True):
+    params_compat = partial(params_compatible, check_annotations=check_annotations)
+    return all(starmap(params_compat, dzip(impl_keywords, iface_keywords).values()))
+
+
+def annotations_compatible(impl, iface):
+    """Check whether the type annotations of an implementation are compatible with
+    the annotations of the interface it implements.
+    """
+    return impl.annotation == iface.annotation