boltons icon indicating copy to clipboard operation
boltons copied to clipboard

Adding parameters with wraps()

Open epruesse opened this issue 5 years ago • 8 comments

It would be nice to be able to do add parameters to a wrapped function, rather than just removing:

def add_arg_extra_arg(func):
  @wraps(func)
  def wrapper(*args, extra_arg=None, **kwargs):
    if extra_arg is not None:
       print("we have an extra argument:", extra_arg)
    func(*args, **kwargs)
  return wrapper

@add_arg_extra_arg()
def myfunc(arg1, arg2, arg3='default'):
    print("arg1", arg1)    
    print("arg2", arg2)
    print("arg3", arg3)

myfunc('val1', 'val2', extra_arg='extra!')

Here's a (hacked) demo implementation (no safeguards, etc):

def wraps(func):
    fb = FunctionBuilder.from_func(func)
    def wrapper_wrapper(wrapper_func):
        fb_wrapper = FunctionBuilder.from_func(wrapper_func)
        fb.kwonlyargs += fb_wrapper.kwonlyargs
        fb.kwonlydefaults.update(fb_wrapper.kwonlydefaults)
        fb.body = 'return _call(%s)' % fb.get_invocation_str()
        execdict = dict(_call=wrapper_func, _func=func)
        fully_wrapped = fb.get_func(execdict)
        fully_wrapped.__wrapped__ = func
        return fully_wrapped

Instead of ignoring the arguments to the wrapper and assuming it takes the same arguments as the wrapped callable, the wraps() decorator could merge the signatures of the callable to be wrapped and the wrapper.

Yes - rare cases where this is needed. An example might be access checking, where the access token is not required inside the wrapped function.

epruesse avatar Feb 08 '19 23:02 epruesse

Hey @epruesse! Merging two signatures sounds pretty cool to me! Kind of tricky with argument orderings and such, but I think some reasonable default behavior should be possible, as you've demonstrated with wrapper_wrapper. I'll see about whipping something up. :)

mahmoud avatar Feb 10 '19 19:02 mahmoud

Great! The devil will be in the details and corner cases I'm afraid.

To stay pythonic and keep the code lean, it might actually be necessary to implement this only for kwonlyargs and prohibit overwriting existing names.

epruesse avatar Feb 10 '19 21:02 epruesse

Best I could do for the recent release (19.0.0) was FunctionBuilder.add_arg().

While that technically fulfills the title of this post. I'm still interested in the merge, still mulling it over. Maybe you could share a more fleshed-out example of how this might be used?

mahmoud avatar Feb 14 '19 19:02 mahmoud

From https://github.com/bioconda/bioconda-utils/blob/613aadb1c042fc81b8e1add17fa632e69b30006f/bioconda_utils/cli.py#L46-L61


def enable_logging(default_loglevel='info'):
    """Adds the parameter ``--loglevel`` and sets up logging

    Args:
      default_loglevel: loglevel used when --loglevel is not passed
    """
    def decorator(func):
        @arg('--loglevel', help="Set logging level (debug, info, warning, error, critical)")
        @utils.wraps(func)
        def wrapper(*args, loglevel=default_loglevel, **kwargs):
            utils.setup_logger('bioconda_utils', loglevel)
            func(*args, **kwargs)
        return wrapper
    return decorator

def enable_debugging():
    """Adds the paremeter ``--pdb`` (or ``-P``) to enable dropping into PDB"""
    def decorator(func):
        @arg('-P', '--pdb', help="Drop into debugger on exception")
        @utils.wraps(func)
        def wrapper(*args, pdb=False, **kwargs):
            try:
                func(*args, **kwargs)
            except Exception as e:
                if pdb:
                    import pdb
                    pdb.post_mortem()
                else:
                    raise
        return wrapper
    return decorator


@arg('filename', help='the name of the file')
@enable_debugging
@enable_logging
def cmd(filename):
  with open(filename):
      pass

def main():
  argh.dispatch_commandss([cmd])

epruesse avatar Feb 14 '19 21:02 epruesse

Since argh inspects the argspec, transparently adding common things to some of the defined comands needs keeping the argspec intact.

epruesse avatar Feb 14 '19 22:02 epruesse

I know - it's a little easier to do this with click, but in that use case, switching CLI libs wasn't really on my list.

epruesse avatar Feb 14 '19 22:02 epruesse

BTW: there seems to be an issue wrapping callables that have type annotations. I got errors while compiling the new stub. Using the quoted style (def x(arg: "MyType"):) fixed it.

epruesse avatar Feb 17 '19 05:02 epruesse

Hmm, I thought we actually hit the non-string annotation pattern in this test. This approach seemed to work well enough for itamar in #203, too. Would you mind providing an example (and the particular error)?

mahmoud avatar Feb 28 '19 08:02 mahmoud