Conversation
…tion signature inspection. Added test to ensure all callables are covered.
| raise | ||
|
|
||
| wrapper = func | ||
| else: |
There was a problem hiding this comment.
elif is_awaitable(func):
# ...
else:
# ...maybe?
There was a problem hiding this comment.
This conditional (and related else) is to check the number of args the receiving function expects.
TL;DR do you have:
def handler(event):
...
or
def handler():
...
Inside each branch is where the check for is_awaitable happens. Of course, it could have been done the other way around... but I didn't want to change the original code, just remove the "hack" for MicroPython now it's not needed.
There was a problem hiding this comment.
I understand what it does and I've mentioned it was a nit ... but these two snippets are identical, the latter one is just (big imho) more logical:
current
def decorator(func):
sig = inspect.signature(func)
if sig.parameters:
if is_awaitable(func):
async def wrapper(event):
return await func(event)
else:
wrapper = func
else:
if is_awaitable(func):
async def wrapper(*args, **kwargs):
return await func()
# this is the last possible branch
else:
def wrapper(*args, **kwargs):
return func()proposed
def decorator(func):
sig = inspect.signature(func)
if sig.parameters:
if is_awaitable(func):
async def wrapper(event):
return await func(event)
else:
wrapper = func
elif is_awaitable(func):
async def wrapper(*args, **kwargs):
return await func()
else:
def wrapper(*args, **kwargs):
return func()anyway, as the logic is identical I am OK with this merged in, after all it might be a subjective style I just like to keep if/elses linear in the indentation whenever I can 😇
WebReflection
left a comment
WebReflection
left a comment
There was a problem hiding this comment.
minor nit you can ignore otherwise looks good to me
2 participants
Description
MicroPython didn't support
inspect.signatureuntil now. It meant we needed to "hack" around this fact. Now MicroPython supportsinspect.signaturethis work removes the "hacky" code and adds a test to ensure MP continues to work as expected.Changes
Checklist
make buildworks locally.