Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: check that method parameters match #35

Closed
wadetregaskis-linkedin opened this issue Jan 4, 2020 · 7 comments
Closed

Comments

@wadetregaskis-linkedin
Copy link

It'd be fantastic if the checks performed by @OverRide could be extended to include validation of the parameters, to ensure they are the same (with certain exceptions such as the use of *args & **kwargs) and have the same type information (where present).

@ashwin153
Copy link
Collaborator

ashwin153 commented Apr 22, 2021

Here is an implementation of this functionality using typing_utils. I'd be happy to put up a PR that introduces this functionality if you are comfortable with the implementation. By calling is_compatible(superclass_method, subclass_method) you would be able to validate that the signature of subclass_method is compatible with superclass_method. @mkorpela

import inspect
from typing import Callable

import typing_utils


def is_compatible(
    x: Callable,
    y: Callable,
) -> None:
    """Verify that the signature of `y` is compatible with the signature of `x`.

    Ensures that any call to `x` will work on `y` by checking the following criteria:

    1. The return type of `y` is a subtype of the return type of `x`.
    2. All parameters of `x` are present in `y`.
    3. All positional parameters of `x` appear in the same order in `y`.
    4. All parameters of `x` are a subtype of the corresponding parameters of `y`.
    5. All parameters of `y` are present in `x`, unless `x` has a `*args` or `**kwargs` parameter.

    :param x: Function to check compatibility with.
    :param y: Function to check compatibility of.
    """
    x_sig = inspect.signature(x)
    y_sig = inspect.signature(y)

    # Verify that the return type of `y` is a subtype of `x`.
    if x_sig.return_annotation != inspect.Signature.empty \
        and y_sig.return_annotation != inspect.Signature.empty \
        and not typing_utils.issubtype(y_sig.return_annotation, x_sig.return_annotation):
        raise TypeError(f"`{y_sig.return_annotation}` is not a `{x_sig.return_annotation}`.")

    # Verify that all parameters in `x` are specified in `y` and that their types are compatible.
    x_var_args = False
    x_var_kwargs = False

    for x_index, (name, x_param) in enumerate(x_sig.parameters.items()):
        if x_param.kind == inspect.Parameter.VAR_POSITIONAL:
            x_var_args = True
        elif x_param.kind == inspect.Parameter.VAR_KEYWORD:
            x_var_kwargs = True
        elif name not in y_sig.parameters:
            raise TypeError(f"`{name}` is not present.")
        else:
            y_index = list(y_sig.parameters.keys()).index(name)
            y_param = y_sig.parameters[name]
            
            if x_param.kind != y_param.kind:
                raise TypeError(f"`{name}` is not `{x_param.kind.description}`")
            elif x_param.kind != inspect.Parameter.KEYWORD_ONLY and x_index != y_index:
                raise TypeError(f"`{name}` is not parameter `{x_index}`")
            elif x_param.annotation != inspect.Parameter.empty \
                and y_param.annotation != inspect.Parameter.empty \
                and not typing_utils.issubtype(x_param.annotation, y_param.annotation):
                raise TypeError(f"`{name} must be a supertype of `{x_param.annotation}`")

    # Verify that no parameters are specified in `y` that are not specified in `x`.
    for name, y_param in y_sig.parameters.items():
        if name not in x_sig.parameters \
            and not (y_param.kind == inspect.Parameter.KEYWORD_ONLY and x_var_kwargs) \
            and not (y_param.kind == inspect.Parameter.VAR_KEYWORD and x_var_kwargs) \
            and not (y_param.kind == inspect.Parameter.VAR_POSITIONAL and x_var_args) \
            and not (y_param.kind == inspect.Parameter.POSITIONAL_ONLY and x_var_args) \
            and not (y_param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD and x_var_args):
            raise TypeError(f"`{name}` is not a valid parameter.")

Here are some examples that demonstrate this functionality.

def foo(x) -> int:
    pass

def bar(x) -> str:
    pass

is_compatible(foo, bar)
# TypeError: `<class 'str'>`` is not a `<class 'int'>`.
def foo(x, y):
    pass

def bar(y, x):
    pass

is_compatible(foo, bar)
# TypeError: `x` is not parameter `0`
def foo(x, *, y):
    pass

def bar(x, y):
    pass

is_compatible(foo, bar)
# TypeError: `y` is not `keyword-only`
def foo(x: int):
    pass

def bar(x: str):
    pass

is_compatible(foo, bar)
# TypeError: `x must be a supertype of `<class 'int'>`
def foo(x):
    pass

def bar(x, y):
    pass

is_compatible(foo, bar)
# TypeError: `y` is not a valid parameter.
def foo(x):
    pass

def bar(x, *args):
    pass

is_compatible(foo, bar)
# TypeError: `args` is not a valid parameter.
def foo(x, **kwargs):
    pass

def bar(x, y):
    pass

is_compatible(foo, bar)
# TypeError: `y` is not a valid parameter.
def foo(x, *args):
    pass

def bar(x, y):
    pass

is_compatible(foo, bar)
# Ok.
def foo(x, **kwargs):
    pass

def bar(x, *, y):
    pass

is_compatible(foo, bar)
# Ok.

@ashwin153
Copy link
Collaborator

ashwin153 commented Apr 22, 2021

Note that this would be a breaking change to EnforceOverrides, because we would be validating signatures that were not being validated before. Therefore, it should be released in a new major version if accepted. If you are uncomfortable with making breaking changes, a EnforceAndValidateOverrides class is a reasonable alternative.

@ashwin153
Copy link
Collaborator

ashwin153 commented Apr 22, 2021

The typing_utils library claims to be compatible with Python 3.6+, and it has integration tests that cover Python 3.6, 3.7, 3.8, and 3.9 to back up this claim. Since overrides supports Python 3.6+, this change should not reduce the compatibility of overrides.

@mkorpela
Copy link
Owner

Thanks @ashwin153 !

@mkorpela
Copy link
Owner

This will be in the next release.

@ashwin153
Copy link
Collaborator

It could be cool to post on r/programming after the release. This library is a big win for static typing in Python IMO; any time that you use abc.abstractmethod you should be using overrides.EnforceOverrides. Glad I could help!

@mkorpela
Copy link
Owner

I think we still need this #56 to actually make a difference. It will then be a new major version again .. so 5.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants