Skip to content

Replacement for functools.wraps that preserves signature#2849

Merged
embray merged 8 commits into
astropy:masterfrom
embray:utils/wraps
Aug 15, 2014
Merged

Replacement for functools.wraps that preserves signature#2849
embray merged 8 commits into
astropy:masterfrom
embray:utils/wraps

Conversation

@embray

@embray embray commented Aug 11, 2014

Copy link
Copy Markdown
Member

Motivated by this comment I added the make_func_with_sig function to astropy.utils.misc. This enabled me to also write a replacement for functools.wraps that copies the wrapped function's argument signature. Turns out there aren't yet as many places where this is useful as I thought there might be. But it could be useful in the future.

@embray

embray commented Aug 11, 2014

Copy link
Copy Markdown
Member Author

This could be used to fix #2367 as well.

@embray embray added the utils label Aug 11, 2014
@embray embray added this to the v1.0.0 milestone Aug 11, 2014
embray added 2 commits August 13, 2014 12:30
<astropy#2835 (comment)>
this adds the make_func_with_sig function to astropy.utils.misc.
Taking advantage of this, a new astropy.utils.compat.misc.wraps
function is provided as a replacement for functools.wraps, which
provides the same functionality but while also preserving the original
wrapped function's signature (so that it displays properly in help())
… thought. And most of them are in utilties/backwards compatibility features that aren't terribly important. But I found this one case where the new wraps implementation is useful. And there could be others in the future.
@embray

embray commented Aug 13, 2014

Copy link
Copy Markdown
Member Author

Rebased.

@embray

embray commented Aug 14, 2014

Copy link
Copy Markdown
Member Author

Looks like I might need to fuss with this a bit more. I found some ways in which this seems to break in some cases.

embray added 4 commits August 14, 2014 16:22
…_sig, rather than only allowing the name of the wrapped function to be used
…eplacement, such that the new function gets the same *name* as the function being wrapped as well
… number into the module make_func_with_sig was called from, and a test to prove that it works.
@astrofrog

Copy link
Copy Markdown
Member

@embray - so I tried using functools.wraps from Python 3.4 in the following example:

from functools import wraps

def dec(func):
    @wraps(func)
    def wrapper(*args, **kwargs):
        return func(*args, **kwargs)
    return wrapper

@dec
def test(a, b=2):
    """
    Test function
    """
    pass

and when doing test?, the signature is wrong, but when using help(test), it is correct:

In [7]: test?
Type:        function
String form: <function test at 0x106961bf8>
File:        /Users/tom/tmp/test_wraps.py
Definition:  test(*args, **kwargs)
Docstring:   Test function

In [8]: help(test)
Help on function test in module __main__:

test(a, b=2)
    Test function

Is this true also for the workarounds you have here for earlier versions?

@astrofrog

Copy link
Copy Markdown
Member

@embray - on the other hand, if I use the decorator package:

from decorator import decorator

@decorator
def dec(func, *args, **kwargs):
    return func(*args, **kwargs)

@dec
def test(a, b=2):
    """
    Test function
    """
    pass

the function signature is correct when doing test?:

In [2]: test?
Type:        function
String form: <function test at 0x1075b7268>
File:        /Users/tom/tmp/test_wraps.py
Definition:  test(a, b=2)
Docstring:   Test function

Maybe we can see what the decorator module does, or maybe even include it in extern and make use of it? What do you think?

(edited example above)

@astrofrog

Copy link
Copy Markdown
Member

By the way, good timing, this will be useful for #2855 :)

@embray

embray commented Aug 14, 2014

Copy link
Copy Markdown
Member Author

My code does a lot of the same stuff as the decorator module, but is a little simpler and IMO easier to understand.

The whole thing here was that on Python 3.4 I was not replacing functools.wraps because help() already works correctly there (as opposed to on previous versions). I didn't realize that the IPython ? syntax did anything special there--I assumed it was delegating to calling help through the underlying kernel. But maybe not?

I could just extend my version of wraps to work on Python 3.4 as well, in which case I might as well move it out of compat and just call it astropy.utils.wraps.

@embray

embray commented Aug 14, 2014

Copy link
Copy Markdown
Member Author

FWIW if I extended my version of wraps to work on Python 3.4 the test? syntax produced the correct output with the faked signature:

In [5]: test?
Type:        function
String form: <function test at 0x7f426e67c048>
File:        /internal/1/root/src/astropy/astropy/<ipython-input-4-08ff0337b56d>
Definition:  test(a, b=2)
Docstring:   Test function

So it looks like however ? is implemented in IPython it doesn't know how to handle the __wrapped__ attribute added to functions by functools.wraps on Python 3.4. I'll see if I can figure out where to put that in IPython and send them a PR...?

@embray

embray commented Aug 14, 2014

Copy link
Copy Markdown
Member Author

This fixes the ? issue on IPython: https://github.com/embray/ipython/compare/ipython:master...embray:help-signatures?expand=1 I'll submit a pull request later once I've had time to clean out the accidental whitespace changes

(though really IPython needs a whitespace cleanup in general)

@astrofrog

Copy link
Copy Markdown
Member

@embray - great! so for now would it make sense to extend your version of wraps to Python 3.4 for now? Or we could just mark it as a known issue?

@astrofrog

Copy link
Copy Markdown
Member

@embray - in any case, this works great on Python <3.4, so 👍 from me! I'd like to use this in #2855.

@embray

embray commented Aug 15, 2014

Copy link
Copy Markdown
Member Author

I'm thinking for now of extending it for use in Python 3.4 as well, just for consistency's sake. I would also like to do a little performance testing of this, though I don't think its impact is going to be enormous in most areas since in most cases it's a brief one time cost.

…mplementation available on Python 3.4 as well. As such I moved it out of astropy.utils.compat since it's really providing a new function rather than some backward/forward compatibility.
embray added a commit to embray/astropy that referenced this pull request Aug 15, 2014
….utils.misc was getting a bit cluttered, so this introduces a few new astropy.utils modules so that astropy.utils.misc could be broken up a bit.
embray added a commit that referenced this pull request Aug 15, 2014
Replacement for functools.wraps that preserves signature
@embray embray merged commit 196071b into astropy:master Aug 15, 2014
@embray embray deleted the utils/wraps branch August 15, 2014 17:43
embray added a commit to embray/astropy that referenced this pull request Aug 19, 2014
…onger needs to include its own copy of the make_func_with_sig utility. I also renamed make_func_with_sig to make_function_with_signature as suggested here: astropy#2835 (comment)  Previously I thought the unabbreviated name was a little on the long side.  But on second thought I prefer the clarity here.
embray added a commit to embray/astropy that referenced this pull request Aug 19, 2014
…onger needs to include its own copy of the make_func_with_sig utility. I also renamed make_func_with_sig to make_function_with_signature as suggested here: astropy#2835 (comment)  Previously I thought the unabbreviated name was a little on the long side.  But on second thought I prefer the clarity here.
embray added a commit to embray/astropy that referenced this pull request Sep 3, 2014
…onger needs to include its own copy of the make_func_with_sig utility. I also renamed make_func_with_sig to make_function_with_signature as suggested here: astropy#2835 (comment)  Previously I thought the unabbreviated name was a little on the long side.  But on second thought I prefer the clarity here.
embray added a commit to embray/astropy that referenced this pull request Sep 24, 2014
…onger needs to include its own copy of the make_func_with_sig utility. I also renamed make_func_with_sig to make_function_with_signature as suggested here: astropy#2835 (comment)  Previously I thought the unabbreviated name was a little on the long side.  But on second thought I prefer the clarity here.
embray added a commit to embray/astropy that referenced this pull request Sep 30, 2014
…onger needs to include its own copy of the make_func_with_sig utility. I also renamed make_func_with_sig to make_function_with_signature as suggested here: astropy#2835 (comment)  Previously I thought the unabbreviated name was a little on the long side.  But on second thought I prefer the clarity here.
embray added a commit to embray/astropy that referenced this pull request Oct 2, 2014
…onger needs to include its own copy of the make_func_with_sig utility. I also renamed make_func_with_sig to make_function_with_signature as suggested here: astropy#2835 (comment)  Previously I thought the unabbreviated name was a little on the long side.  But on second thought I prefer the clarity here.
embray added a commit to embray/astropy that referenced this pull request Oct 15, 2014
…onger needs to include its own copy of the make_func_with_sig utility. I also renamed make_func_with_sig to make_function_with_signature as suggested here: astropy#2835 (comment)  Previously I thought the unabbreviated name was a little on the long side.  But on second thought I prefer the clarity here.
embray added a commit to embray/astropy that referenced this pull request Oct 21, 2014
…onger needs to include its own copy of the make_func_with_sig utility. I also renamed make_func_with_sig to make_function_with_signature as suggested here: astropy#2835 (comment)  Previously I thought the unabbreviated name was a little on the long side.  But on second thought I prefer the clarity here.
embray added a commit to embray/astropy that referenced this pull request Nov 12, 2014
…onger needs to include its own copy of the make_func_with_sig utility. I also renamed make_func_with_sig to make_function_with_signature as suggested here: astropy#2835 (comment)  Previously I thought the unabbreviated name was a little on the long side.  But on second thought I prefer the clarity here.
ntessore pushed a commit to glass-dev/cosmology that referenced this pull request Dec 7, 2020
… astropy.utils.misc was getting a bit cluttered, so this introduces a few new astropy.utils modules so that astropy.utils.misc could be broken up a bit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants