Nathaniel Smith <n...@pobox.com> added the comment:
There are two different axes of sync/async here, and it's important not to mix
them up: the context manager could be sync or async, and the function that's
being decorated could be sync or async. This issue is about the case where you
have a sync context manager, and it's decorating an async function.
The current behavior is definitely super confusing and not at all useful.
In general, I'm also very skeptical about using iscoroutinefunction to
automatically change semantics, because of the issue Andrew mentions about
failing to handle complicated objects like functools.partial instances or sync
functions that return awaitables. In this *specific* case though, I'm not
sure... maybe it actually is OK?
What's special about this case is that when you write:
@sync_cm()
async def foo():
...
...the same person is writing both the @ line and the 'async def' line, and
they're right next to each other in the source code. So the normal concern
about some unexpected foreign object being passed in doesn't really apply. And
in fact, if I did have an awaitable-returning-function:
@sync_cm()
def foo():
return some_async_fn()
...then I think most people would actually *expect* this to be equivalent to:
def foo():
with sync_cm():
return some_async_fn()
Not very useful, but not surprising either. So maybe this is the rare case
where switching on iscoroutinefunction is actually OK? The only cases where it
would fail is if someone invokes the decorator form directly, e.g.
new_fn = sync_cm()(old_fn)
This seems pretty rare, but probably someone does do it, so I dunno.
If we think that's too magic, another option would be to make up some more
explicit syntax, like:
@sync_cm().wrap_async
async def foo(): ...
...Oh, but I think this breaks the Guido's gut feeling rule
(https://mail.python.org/pipermail/python-dev/2004-August/046711.html), so it
would have to instead be:
@sync_cm.wrap_async()
async def foo(): ...
And even if we decide not to automatically switch on iscoroutinefunction, we
should probably add a warning or something at least, because right now
@sync_cm()
async def ...
is definitely a mistake.
(Adding Nick to nosy as the contextlib maintainer.)
----------
nosy: +ncoghlan, njs
_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue37398>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
