Skip to content

Simplifying the implementation of signature-overloaded functions #7966

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

Closed
anntzer opened this issue Jan 28, 2017 · 5 comments
Closed

Simplifying the implementation of signature-overloaded functions #7966

anntzer opened this issue Jan 28, 2017 · 5 comments

Comments

@anntzer
Copy link
Contributor

anntzer commented Jan 28, 2017

There are many functions in matplotlib that have "interesting" call signatures, e.g. that could be called with 1, 2 or 3 arguments with different semantics (i.e. not just binding arguments in order and having defaults for the later ones). In this case, the binding of arguments is typically written on an ad-hoc basis, with some bugs (e.g. the one I fixed in https://github.com/matplotlib/matplotlib/pull/7859/files#diff-84224cb1c8cd1f13b7adc5930ee2fc8fR365) or difficult to read code (e.g. https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/quiver.py#L374 (quiver._parse_args)).

Moreover, there are also cases where a function argument should be renamed, but cannot be due to backwards compatibility considerations (e.g. #7954).

I propose to fix both issues using a signature-overloading decorator (see below for prototype implementation). Basically, the idea would be to write something like

@signature_dispatch
def func(<inner_signature>): ... # inner function definition

@func.overload
def func(<signature_1>):
    # <play with args until they match inner_signature>
    return func.__wrapped__(<new_args>)  # Refers to the "inner" function

@func.overload
def func(<signature_2>):
    # <play with args until they match inner_signature>
    return func.__wrapped__(<new_args>)  # Refers to the "inner" function

where the first overload that can bind the arguments is the one used.

In order to support changes in signature due to argument renaming, an overload with the previous signature that raises a DeprecationWarning before forwarding the argument to the "inner" function can be used.

Thoughts?

Protoype implementation:

"""A signature dispatch decorator.

Decorate a function using::

    @signature_dispatch
    def func(...):
        ...

and provide signature overloads using::

    @func.overload
    def func(...): # Note the use of the same name.
        ...
        # Refer to the "original" function as ``func.__wrapped__``

Calling the function will try binding the arguments passed to each overload in
turn, until one binding succeeds; that overload will be called and its return
value (or raised Exception) be used for the original function call.

Overloads can define keyword-only arguments in trailing position in a Py2
compatible manner by having a marker argument named ``__kw_only__``; that
argument behaves like "*" in Py3, i.e., later arguments become keyword-only
(and the ``__kw_only__`` argument itself is always bound to None).

Overloads can define positional arguments in leading position (as defined in
https://docs.python.org/3/library/inspect.html#inspect.Parameter.kind) by
having a marker argument named ``__pos_only__``; earlier arguments become
positional-only (and the ``__pos_only__`` argument iself is always bound to
None).

This implementation should be compatible with Py2 as long as a backport of the
signature object (e.g. funcsigs) is used instead.
"""


from collections import OrderedDict
from functools import wraps
from inspect import signature


def signature_dispatch(func):

    def overload(impl):
        sig = signature(impl)
        params = list(sig.parameters.values())
        try:
            idx = next(idx for idx, param in enumerate(params)
                       if param.name == "__pos_only__")
        except ValueError:
            pass
        else:
            # Make earlier parameters positional only, skip __pos_only__ marker.
            params = ([param.replace(kind=param.POSITIONAL_ONLY)
                       for param in params[:idx]]
                      + params[idx + 1:])
        try:
            idx = next(idx for idx, param in enumerate(params)
                       if param.name == "__kw_only__")
        except ValueError:
            pass
        else:
            # Make later parameters positional only, skip __kw_only__ marker.
            params = (params[:idx]
                      + [param.replace(kind=param.KEYWORD_ONLY)
                         for param in params[idx + 1:]]

        sig = sig.replace(parameters=params)
        impls_sigs.append((impl, sig))
        return wrapper

    @wraps(func)
    def wrapper(*args, **kwargs):
        for impl, sig in impls_sigs:
            try:
                ba = sig.bind(*args, **kwargs)
            except TypeError:
                continue
            else:
                if "__pos_only__" in signature(impl).parameters:
                    ba.arguments["__pos_only__"] = None
                return impl(**ba.arguments)
        raise TypeError("No matching signature")

    impls_sigs = []
    wrapper.overload = overload
    return wrapper


@signature_dispatch
def slice_like(x, y, z):
    return slice(x, y, z)

@slice_like.overload
def slice_like(x, __pos_only__):
    return slice_like.__wrapped__(None, x, None)

@slice_like.overload
def slice_like(x, y, __pos_only__):
    return slice_like.__wrapped__(x, y, None)

@slice_like.overload
def slice_like(x, y, z, __pos_only__):
    return slice_like.__wrapped__(x, y, z)

assert slice_like(10) == slice(10)
assert slice_like(10, 20) == slice(10, 20)
assert slice_like(10, 20, 30) == slice(10, 20, 30)
try: slice_like(x=10)
except TypeError: pass
else: assert False
@efiring
Copy link
Member

efiring commented Jan 28, 2017

I see the attraction to something like this, but a contrary view is that it is adding a fair-size chunk of extraordinarily complex, hard-to-understand code (your decorator machinery) in order to replace existing code that mostly works fine with a similar amount of code. So it looks to me like it adds complexity and LOC for no real gain.

@WeatherGod
Copy link
Member

Actually, I kind of like this idea. One of the things that has driven me nuts with the "chameleon functions" is that the argument mucking is often times intertwined with the function logic. This helps to separate it out. And the ability to easily perform deprecations on specific call signatures is also valuable.

Now, the big questions is... is pycharms going to hate us even more?

@tacaswell tacaswell added this to the 2.2 (next next feature release) milestone Aug 27, 2017
@tacaswell
Copy link
Member

This might make pycharm hate us less that *args, **kwargs.

@anntzer
Copy link
Contributor Author

anntzer commented Aug 27, 2017

As a side note, a possibly simpler implementation (but without support for kwonly args on Py2 (other than manually, as usual)) was proposed in #6404 (comment).

@anntzer
Copy link
Contributor Author

anntzer commented Dec 16, 2022

This is basically now in as _api.select_matchin_signature, and one could e.g. write

diff --git i/lib/matplotlib/quiver.py w/lib/matplotlib/quiver.py
index c6e51afc0a..23e64b4194 100644
--- i/lib/matplotlib/quiver.py
+++ w/lib/matplotlib/quiver.py
@@ -404,21 +404,17 @@ def _parse_args(*args, caller_name='function'):
     caller_name : str
         Name of the calling method (used in error messages).
     """
-    X = Y = C = None
-
-    nargs = len(args)
-    if nargs == 2:
-        # The use of atleast_1d allows for handling scalar arguments while also
-        # keeping masked arrays
-        U, V = np.atleast_1d(*args)
-    elif nargs == 3:
-        U, V, C = np.atleast_1d(*args)
-    elif nargs == 4:
-        X, Y, U, V = np.atleast_1d(*args)
-    elif nargs == 5:
-        X, Y, U, V, C = np.atleast_1d(*args)
-    else:
-        raise _api.nargs_error(caller_name, takes="from 2 to 5", given=nargs)
+    args = [*map(np.atleast_1d, args)]  # handle both scalars and masked arrays
+    try:
+        X, Y, U, V, C = _api.select_matching_signature([
+            lambda U, V: (None, None, U, V, None),
+            lambda U, V, C: (None, None, U, V, C),
+            lambda X, Y, U, V: (X, Y, U, V, None),
+            lambda X, Y, U, V, C: (X, Y, U, V, C),
+        ], *args)
+    except TypeError:
+        raise _api.nargs_error(
+            caller_name, takes="from 2 to 5", given=len(args)) from None
 
     nr, nc = (1, U.shape[0]) if U.ndim == 1 else U.shape
 

but let's consider this only for cases where the helper is really needed (which correspond to the current places where select_matching_signature is indeed used).

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

4 participants