Proposal: signature copying for kwargs.

[dmoisset]

There's a quite common pattern in python code which is:

def function(foo, *args, **kwargs):
    # do something with foo
    other_function(*args, **kwargs)
    # possibly do something else

def other_function(color: str=..., temperature: float=..., style: Stylesheet=..., timeout: Optional[int]=..., database_adaptor: Adaptor=..., strict: bool=..., output: IO[str], allow_frogs: bool=..., mode: SomeEnum=...):
    # do something with a lot of options

(a usual subcase of this one is when other_function is actually super().function ). This presents two problems for a static analyzer:

• the call from function to other_function can not be type-checked properly because of the *args, **kwargs in the call arguments.
• there is no sensible way to annotate function, so calls to it are unchecked.

The problem for me also affects readability of the code (which is for me one of the main problems that annotations tries to address). James Powell from numfocus even gave a pydata talk about the difficulties it brings at https://www.youtube.com/watch?v=MQMbnhSthZQ

Even if theoretically the args/kwargs packing feature of python can be used with more or less arbitrary data, IMO this use-case is common enough to warrant some special treatment. I was thinking on a way to flag this usage, for example:

@delegate_args(other_function)
def function(foo, *args, **kwargs):
    other_function(*args, **kwargs)

This could hint an analyzer so:

• On calls to function, the "extra" arguments are checked to match the signature of other_function
• The call to other_function is considered valid given that it uses the same arguments (I know that the code above could have modified the content of kwargs, but it's still more checking than what we have now).

For me, even without static analyzer, the readability benefits of seeing

@delegate_args(matplotlib.pyplot.plot)
def plot_valuation(ticker_symbol: str, start: date, end: date, *args, **kwargs): ...

and knowing that plot_valuation accepts any valid arguments from matplotlib's plot function, is worth it.

[gvanrossum]

Gotta make this quick: Yes, we experience this in our own (Dropbox) code bases a lot too. And I agree this is useful for readability even without static checking. I think there are some details to be worked out -- what I often see is that the wrapping function adds some fixed args to the function in delegates too, so those should not be accepted:

def raw(name='me', age=42): ...

def cooked(first_name, last_name, **extra):
    raw(name=first_name + ' ' + last_name, **extra)

cooked(age=100)  # OK
cooked(name='Guido')  # Error

Also of course the wrapper may also have some of its own arguments that are unrelated to the wrapped function.

[JukkaL]

I've seen this pattern quite frequently as well, and it would be nice to support it somehow. Refactoring existing code to use explicitly spelled out individual arguments instead of *args and **kwargs can be lot of work and make the code harder to maintain, especially if there are many args to propagate.

Expanding on the decorator idea, maybe it should be possible to declare that a function accepts all positional and keyword args another function accepts, except for a set of args. Example:

def raw(name='me', age=42): ...

@delegate_args(raw, except=['name'])
def cooked(first_name, last_name, **extra):
    raw(name=first_name + ' ' + last_name, **extra)

cooked(age=100)  # OK
cooked(name='Guido')  # Error

Implementing this in a type checker looks a little tricky, especially when using both *args and **kwargs. Special-casing the whole thing looks possible, but sometimes programmers mutate the **kwargs dictionary and type checking that would likely require some sort of support for "dict-as-struct".

[dmoisset]

OK, to support a bit of this with data I did a small experiment: take a random sample of stdlib files until I got about 100 function definitions with open keyword args; then another random sample in django with 100 more defs, and check manually what they do with the arguments. My finding say that:

1. 17% of the defs with **k are for functions that never use k
2. 18% of the defs with **k actually use k as a dictionary (calling get(), pop(), iterating on it, etc., but also storing it into a variable or attribute, or passing k as a parameter but without **)
3. 40% of the defs with **k pass it as is to another function, without adding more positional or keyword arguments on the call (This percentage also includes the functions that pass *varargs as is too).
4. 24% of the defs with **k pass it (with ** again) to another function, but the call includes some extra arguments (positional or keyword).
5. 1% of the defs with **k use it for self.__dict__.update(k).

Case 1 is trivial and works well already with Any or object. Case 2 is the hard one (when the dictionary is heterogeneous, which appears to be the most frequent scenario, although I didn't collect that data). Case 5 could be a verification target for a type checker but seems quite unusual. My proposal would cover case 3, and with @JukkaL addition, a part of case 4.

So "hard" cases would be reduced to about 20% of the def statements with **kwargs according to this semi-scientific experiment :) .

From what I saw of case 4, actually I think that a lot of cases could be covered automatically. The example that @JukkaL posted is not very common. The common case for those (at least in my sample) are the following two:

def raw(name=..., age=...): ...

@delegate_args(raw)
def cooked(name, **extra):
    raw(name=name.upper(), **extra)
    # here the typechecker could deduce that `**extra` never contains a `name`, because it's a named argument of "cooked"

def raw2(a, b, c, name=..., age=...): ...

@delegate_args(raw)
def cooked2(a, b, c, **extra):
    raw(a+1, b, c, **extra)
    # here the typechecker could deduce that `**extra` never contains `a`, `b`, or `c`, because they are named arguments of `cooked2`

So the except=... proposal is helpful, but in just a handful of cases (which I prefer, and also avoids the "except" keyword). There's also this case (which I included in my group 2) that could benefit from a similar proposal which is:

@delegate_args(raw, include=['foo'])
def cooked(**extra):
    foo = extra.pop('foo', SOME_DEFAULT)
    do_something_with(foo)
    raw( **extra)

I would have declared foo as an explicit argument and perhaps that's what we should recommend, but I'm a bit surprised on how relatively common this pattern is.

[JukkaL]

@dmoisset Thanks for the careful analysis! Having data like this makes decisions much easier. So the conclusion seems to be that this would be useful even without support for dict-as-struct. It might be useful for us to run a similar analysis against Dropbox code to get another data point.

[dfee]

I came across this issue, and wanted to present my solution to the problem:

import forge

def raw(name='he', age=42):
    return f'{name} is {age}'

@forge.compose(
    forge.copy(raw, exclude=('name')),
    forge.insert((forge.arg('first_name'), forge.arg('last_name')), index=0),
)
def cooked(first_name, last_name, **extras):
    extras['name'] = first_name + ' ' + last_name
    return forge.callwith(raw, extras)

(and some tests that validate that it works as expected)

import inspect
assert repr(inspect.signature(cooked)) == '<Signature (first_name, last_name, age=42)>'
assert raw(age=100)  == 'he is 100'
assert raw('Guido', age=42) == 'Guido is 42'
assert cooked('Guido', 'VR', 42) == 'Guido VR is 42'

try:
    cooked(name='Guido')  # Error
except TypeError as exc:
    assert exc.args[0] == "cooked() missing a required argument: 'first_name'"

forge is a package I recently released that allows for signature revision (add, remove, modify parameters as well as convert or validate argument values, etc.).

I'm curious as to whether a mypy plugin could be written that would allow for static analysis.

[alexmojaki]

This is quite the mouthful:

@forge.compose(
    forge.copy(raw, exclude=('name')),
    forge.insert((forge.arg('first_name'), forge.arg('last_name')), index=0),
)
def cooked(first_name, last_name, **extras):

How about instead forge.copy just doesn't interfere with arguments in the function it's decorating that don't have * or **? In other words the forge.insert could be automatic. If somehow that turns out to be a problem in some cases it could be turned off with a keyword argument.

I don't understand the need for this:

    extras['name'] = first_name + ' ' + last_name
    return forge.callwith(raw, extras)

Why not raw(name=first_name + ' ' + last_name, **extra)?

[dfee]

So forge can do more than solve the problem discussed here. You bring up an interesting point though, is that a common-enough problem to which a canned decorator should be provided? I'll share a snippet in a little bit.

The reason for callwith is that reconstituting arguments (especially the ordering of positional or keyword parameters is a non-trivial problem:

import forge

def func(a, b, c, d=4, e=5, f=6, *args):
    return (a, b, c, d, e, f, args)

@forge.sign(
    forge.arg('a', default=1),
    forge.arg('b', default=2),
    forge.arg('c', default=3),
    *forge.args,
)
def func2(*args, **kwargs):
    return forge.callwith(func, kwargs, args)

assert forge.repr_callable(func2) == 'func2(a=1, b=2, c=3, *args)'
assert func2(10, 20, 30, 'a', 'b', 'c') == (10, 20, 30, 4, 5, 6, ('a', 'b', 'c'))

The alternative to that is manual interpolation of argument values which is just as big of a problem:

import forge

def func(a, b, c, d=4, e=5, f=6, *args):
    return (a, b, c, d, e, f, args)

@forge.sign(
    forge.arg('a', default=1),
    forge.arg('b', default=2),
    forge.arg('c', default=3),
    *forge.args,
)
def func2(*args, **kwargs):
    return func(
        kwargs['a'],
        kwargs['b'],
        kwargs['c'],
        4,
        5,
        6,
        *args,
    )

assert forge.repr_callable(func2) == 'func2(a=1, b=2, c=3, *args)'
assert func2(10, 20, 30, 'a', 'b', 'c') == (10, 20, 30, 4, 5, 6, ('a', 'b', 'c'))

I explain that further, in the docs.

[alexmojaki]

OK, that's cool, but:

1. Anyone who writes

def func(a, b, c, d=4, e=5, f=6, *args):

instead of

def func(a, b, c, *args, d=4, e=5, f=6):

deserves to be slapped, and

2. If I understand correctly, the cooked/raw example can work fine without callwith?

[dfee]

Both are correct :)

[dfee]

Alright, I got a minute to draft out an extend revision:

import functools
import inspect

import forge


class extend(forge.Revision):
    """
    Extends a function's signature...
    """
    def __init__(self, callable, *, include=None, exclude=None):
        # pylint: disable=W0622, redefined-builtin
        self.callable = callable
        self.include = include
        self.exclude = exclude

    def revise(self, previous):
        extensions = forge.fsignature(self.callable)
        if self.include:
            extensions = list(forge.findparam(extensions, self.include))
        elif self.exclude:
            extensions = [
                param for param in extensions
                if param not in forge.findparam(extensions, self.exclude)
            ]

        params = [
            param for param in previous
            if param.kind is not forge.FParameter.VAR_KEYWORD
        ] + list(extensions)

        return forge.FSignature(params)

Usage is straightforward:

def raw(name='he', age=42):
    return f'{name} is {age}'

@extend(raw, exclude=('name'))
def cooked(first_name, last_name, **extras):
    return raw(name=f'{first_name} {last_name}', **extras)

And the tests still pass:

assert repr(inspect.signature(cooked)) == '<Signature (first_name, last_name, age=42)>'
assert raw(age=100)  == 'he is 100'
assert raw('Guido', age=42) == 'Guido is 42'
assert cooked('Guido', 'VR', 42) == 'Guido VR is 42'

try:
    cooked(name='Guido')  # Error
except TypeError as exc:
    assert exc.args[0] == "cooked() missing a required argument: 'first_name'"

The caveats to this approach are that a user must remain wary of the ordering of "parameter kind" and parameters with default values (or use within a @forge.compose(forge.extend(...), forge.sort()) construct.

[JukkaL]

python/mypy#5559 has an implementation of the basic proposal with exclude= by @alexmojaki as a mypy extension. Before moving on, I'd like to discuss the design a bit more.

In particular, there's the question of what syntax to use. All examples above use a decorator, but @ilevkivskyi also suggested (in python/mypy#5559) using a TypedDict (another mypy extension) or a protocol-based callable as an annotation. Here's an example using a callable annotation from the above PR:

class DoStuff(Protocol):
    def __call__(a: int, b: int = ..., c: int = ...) -> None: ...

f: DoStuff
def f(*args, **kwargs):
    ...

g: DoStuff
def g(*args, **kwargs):
    f(*args, **kwargs)

This is not optimal if the original function (such as f above) doesn't use *args and/or **kwargs for all arguments, since then we'd need to duplicate the argument names and types, even though that's what the proposal is trying to avoid. So there would arguably only be a benefit if there are two or more functions that delegate to f.

Here's another idea (which will likely be harder to implement):

def f(x: int, y: str = '') -> None:
    ...

g: TypeOf[f]
def g(*args, **kwargs):
    ...

We'd introduce a TypeOf[x] type operator that is equivalent to the type of the expression within square brackets. It could potentially also be used for other things, such as callbacks, though these use cases may be pretty marginal:

def default(x: int, y: str = '') -> None:
    ...

def do_stuff(cb: TypeOf[default] = default) -> int:
    ...

This would only cover uses cases where the signatures of the two functions are identical.

Even if we continue with the decorator proposal, we don't have an agreement on what we'd call it. The ideas above aren't quite self-explanatory enough, in my opinion. The feature is not very widely useful, so I feel like we should try to make name very clear and explicit. Here are a bunch of random ideas (using a real example from subprocess):

@copy_signature(Popen)
def call(*popenargs, timeout: Optional[float] = None, **kwargs)): ...

@inherit_signature(Popen)
def call(*popenargs, timeout: Optional[float] = None, **kwargs)): ...

@with_signature(Popen)
def call(*popenargs, timeout: Optional[float] = None, **kwargs)): ...

@use_signature(Popen)
def call(*popenargs, timeout: Optional[float] = None, **kwargs)): ...

@use_signature_from(Popen)
def call(*popenargs, timeout: Optional[float] = None, **kwargs)): ...

@apply_signature(Popen)
def call(*popenargs, timeout: Optional[float] = None, **kwargs)): ...

Here my rationale is that the decorator actually does no delegation -- the assumption is that the function delegates to another function, but that's actually not enforced by the feature, I assume. So the effect is to take the signature of another function and apply it to the decorated function.

[ilevkivskyi]

Here are few more random ideas/comments:

• Instead of TypeOf[f], that may give an impression that this applies to any other object, I would propose to allow single argument Callable like this:

  def f(x: int, y: str = '') -> None:
      ...
  g: Callable[f]
  def g(*args, **kwargs):
      ...
  def do_stuff(cb: Callable[f] = f) -> int:
      ...

• ...protocol-based callable as an annotation...
  So there would arguably only be a benefit if there are two or more functions that delegate to f.

  Yes, the same also applies to using TypedDict to annotate **kwargs. Allowing TypedDict in annotations has its own use case, Allow using TypedDict for more precise typing of **kwds mypy#4441, so probably it is a bit orthogonal here.

• If we go with the decorator approach, maybe again we can use (now stalled) @declared_type? (I am not sure I like this however.)

My preference would be probably to go with g: Callable[f].