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

Adding parameters with wraps() #201

Open
epruesse opened this issue Feb 8, 2019 · 8 comments
Open

Adding parameters with wraps() #201

epruesse opened this issue Feb 8, 2019 · 8 comments

Comments

@epruesse
Copy link

epruesse commented Feb 8, 2019

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.

@mahmoud
Copy link
Owner

mahmoud commented Feb 10, 2019

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. :)

@epruesse
Copy link
Author

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.

@mahmoud
Copy link
Owner

mahmoud commented Feb 14, 2019

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?

@epruesse
Copy link
Author

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
Copy link
Author

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

@epruesse
Copy link
Author

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
Copy link
Author

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.

@mahmoud
Copy link
Owner

mahmoud commented Feb 28, 2019

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)?

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

2 participants