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

iscoroutinefunction returns False for wrapper functions with update_wrapper applied #100317

Closed
jonasBoss opened this issue Dec 17, 2022 · 4 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@jonasBoss
Copy link

When decorating a async function with a decorator that uses @functools.wraps or update_wrapper
inspect.iscoroutinefunction returns False.

The following code demonstrates this issue:

import functools
import inspect

async def fn():
    pass

assert inspect.iscoroutinefunction(fn)

def decorator(fn):
    def wrapper(*args, **kwargs):
        return fn(*args, **kwargs)
    return wrapper

assert not inspect.iscoroutinefunction(decorator(fn))

def signature_preserving_decorator(fn):
    @functools.wraps(fn)
    def wrapper(*args, **kwargs):
        return fn(*args, **kwargs)

    return wrapper

assert inspect.iscoroutinefunction(signature_preserving_decorator(fn))
# AssertionError

I would expect the last assert statement to pass

  • CPython versions tested on: 3.10.8
  • Operating system and architecture: Linux 5.19.17-2-MANJARO x86_64 GNU/Linux
@jonasBoss jonasBoss added the type-bug An unexpected behavior, bug, or error label Dec 17, 2022
@jonasBoss jonasBoss changed the title iscoroutinefunction returns False for wrapped functions with update_wrapper applied iscoroutinefunction returns False for wrapper functions with update_wrapper applied Dec 17, 2022
@AlexWaygood AlexWaygood added the stdlib Python modules in the Lib dir label Dec 17, 2022
@JosephSBoyle
Copy link
Contributor

JosephSBoyle commented Dec 18, 2022

I agree that the behaviour you describe is what I would also expect.

Searching for the phrase "def is coroutine" in the source I found this link:
https://github.com/python/cpython/blob/f4c03484da59049eb62a9bf7777b963e2267d187/Misc/NEWS.d/3.5.0b3.rst

" coroutines; inspect.iscoroutine no longer uses collections.abc.Coroutine... it's intended to test for pure 'async def' coroutines only;"

Which might suggest that your use case is not supported. (I think it ought to be).

@carljm
Copy link
Member

carljm commented Dec 20, 2022

#99247 recently added inspect.markcoroutinefunction which allows marking arbitrary functions as coroutine functions even if they were not defined with async def. This can be used manually to handle your situation:

def signature_preserving_decorator(fn):
    @functools.wraps(fn)
    def wrapper(*args, **kwargs):
        return fn(*args, **kwargs)
    if inspect.iscoroutinefunction(fn):
       wrapper = inspect.markcoroutinefunction(wrapper)
    return wrapper

But this is a lot of boilerplate.

This does raise some questions for functools.wraps:

  1. (Narrow version) should it automatically transfer the "mark" from markcoroutinefunction, if present on the wrapped function, to the wrapper?
  2. (Broad version) should it automatically apply markcoroutinefunction to the wrapper anytime the wrapped function passes iscoroutinefunction?

I think we should discard option 1 as breaking the contract of markcoroutinefunction: it is supposed to mean "treat me as if I were an async def," so we should not intentionally introduce cases where a function is treated differently if it is "marked" vs actual async def.

Given that iscoroutinefunction already natively supports functools.partial wrappers around coroutine functions, it seems most consistent to me that functools.wraps would also transparently pass through iscoroutinefunction-ness (i.e. option 2). My only question here is backward-compatibility: option 2 would cause existing wrappers around async functions using functools.wraps to change behavior with iscoroutinefunction. We can only do this if we are confident the existing behavior is a bug, i.e. nobody would want the current behavior. I'm not entirely sure of this.

CCing people actively involved in #99247 @carltongibson @kumaraditya303 @gvanrossum

@gvanrossum
Copy link
Member

Is two lines really that much boilerplate? (If you need it enough you can write a helper that does it in one. :-)

I worry there's a fundamental problem with automatically passing the "iscoroutinefunction" bit in functools.wraps(): what if the decorator is something that takes an async function and produces a sync version of it (e.g. by calling asyncio.run() on it)? I'm not a big user of wraps() myself, but from reading its docs (and those for update_wrapper()) it is far from clear that this would be an inappropriate use for those functions -- the docs mostly talk about things like preserving the function name and docstring.

For this reason I'm not a fan of automatically applying markcoroutinefunction in wraps(). But maybe we could add a new keyword arg to it (and to update_wrapper()) that transfers the flag?

I also note that wrapping an async function in a sync wrapper may not do the right thing. E.g. a wrapper like this typical example:

def logcalls(func):
    def wrapper(*args, **kwds):
        print("args=", args)
        res = func(*args, **kwds)
        print("res=", res)
        return res

would print the "result" immediately when a wrapped async function is called, not when it returns.

@carltongibson
Copy link
Contributor

carltongibson commented Dec 21, 2022

Thanks @carljm.

I have Django's various decorators in the queue for updating to support async views correctly. I think I'd expect that I'd have to write this version that you have (with the additional lines):

def signature_preserving_decorator(fn):
    @functools.wraps(fn)
    def wrapper(*args, **kwargs):
        return fn(*args, **kwargs)
    if inspect.iscoroutinefunction(fn):
       wrapper = inspect.markcoroutinefunction(wrapper)
    return wrapper

I note that if you define wrapper as async def, the example script passes (as expected, I guess):

async def fn():
    pass

def signature_preserving_decorator(fn):
    @functools.wraps(fn)
    async def wrapper(*args, **kwargs):
        return fn(*args, **kwargs)

    return wrapper

assert inspect.iscoroutinefunction(signature_preserving_decorator(fn))

So then we're in the exact use-case for markcoroutinefunction: I need (for reasons not given in this example, but Django's usage is wrapping both sync and async views) to have a sync def function that returns a coroutine object (but I can't just call it and use iscoroutine) so you need to trust me — In that case having to apply markcoroutinefunction by-hand seems reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

6 participants