-
Notifications
You must be signed in to change notification settings - Fork 138
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
✨ Add alternative APIs that improve typing and tooling support (e.g. autocompletion) #403
Conversation
I'd like to see this API in trio before it's implemented in anyio. Ultimately the best outcome here is for typing support for functions like asyncio.to_thread tg.start_soon etc via: P = ParamSpec("P")
R = TypeVar("R")
async def to_thread(fn: Callable[P, R], /, *args: P.args, **kwargs: P.kwargs) -> R: ... |
FYI there's a long open issue in trio about this: python-trio/trio#470 |
Thanks for the feedback! And thanks for the link @uSpike, I hadn't seen that one. @graingert, about the preference for: P = ParamSpec("P")
R = TypeVar("R")
async def to_thread(fn: Callable[P, R], /, *args: P.args, **kwargs: P.kwargs) -> R: ... As Nathaniel explains there, those functions take configs for them, so they can't take kwargs.
2021-12-26 Edit: I realize I'm not proposing the same as this comment, that one proposes the second call to take the function, my proposal is that the first call takes the function, that's how all the And now with Now, I'm not sure it would be easy to convince everyone one way or another in Trio given the huge thread and opinions there, but I'll try. I also thought there was a chance it could make sense to solve/support it here even if Trio hasn't solved it yet, as was the case with #390. |
#390 was easier because it was fairly obvious what the correct behavior there was. This PR involves actual API design choices so it's not so clear cut. |
Get it, thanks @agronholm! I'll comment in the Trio thread and see what happens. |
So...rather than adding these elaborate functions, would it not work to just use a lambda? E.g. |
That's a very good point! Very clever and simple. I don't know why that didn't occur to me. 🤦 I've been trying to see any reason why that would not work and I don't find any, even async functions would seem to work (they just can't be awaited inside the lambda, but if they return the awaitable thing then the lambda becomes an awaitable function 🤷 , so it should work). That part, of calling an async function inside of a lambda but not awaiting it might be a bit mind-bending for newcomers, but it can probably be just documented. I think that's a clever and simple approach that would work for most cases (in particular for the stuff that I'm interested in) and doesn't need any API change. I guess that would lose some introspection stuff as the wrapper and function name is not propagated, but I don't even know when that would be useful. The only caveat I see is that it was not obvious to me, I would expect it to also not be obvious to others. In particular as the current API already receives positional arguments. But maybe just adding that to the docs and explaining it would be good enough for most cases. Now I think it could be nice if none of the APIs received any arguments at all, so that there was only one way to do it, with lambdas, and it was obvious that it was the only way, but I know that's not possible now... anyway, maybe just documenting that approach with lambdas is good enough to achieve what this PR was intended to achieve. 🤔 |
for more information, see https://pre-commit.ci
Are you still pushing this instead of using lambdas? |
Ah, that last commit pushed was just because I had an uncommited typo fix in the same PR. I think you would rather not have this here, right? Would you like/accept a docs PR documenting lambdas and how that would help with types and stuff? In any case, feel free to close this PR if you prefer. |
Yes, I think I'd like that! |
Just remember that there is one ugly catch with lambdas: if you schedule more than one in the same function using the same variables, you may be surprised to find that they all bind to the latest values of said variable. |
✨ Add alternative APIs that improve typing and tooling support (e.g. autocompletion in editors)
Description
I want to propose adding a new API style for three functions (additional to the current API) that would take advantage of new type annotations (PEP 612 - Parameter Specification Variables), to get better tooling and editor support.
In particular, this would allow/support mypy checks, inline editor errors, and ✨ autocompletion ✨ for positional and keyword arguments for:
tg.start_soon()
)to_thread.run_sync()
)from_thread.run()
)This would also allow mypy and editors to provide type checks, autocompletion, etc. for the return values received when sending tasks to a worker thread and when receiving them from a worker thread:
to_thread.run_sync()
from_thread.run()
Usage
The main difference is that instead of having a function that takes the function to call plus:
...this new API only takes the function to call plus configs (like the task name), and then returns another function that takes the positional and keyword arguments. And when that function is called, it does the actual work. So it's used like this:
And now it also supports keyword arguments, not only positional ones, so it's not necessary to use (and understand) partials to use keyword arguments:
Examples
Task Groups
To Thread (asyncify)
From Thread (syncify)
Function names Bikeshedding
I wanted to have short function names that were descriptive enough, that's why the "somethingify".
I thought any more descriptive variant like
generate_async_function_that_calls_run_sync
would be too long. 😅My rationale is that if a user wants to call a blocking function while inside async stuff, the user might want to have a way to "asyncify" that blocking function. And also the "somethingify" doesn't imply it is being executed/called right away, but that something is "converted" in some way. I would hope that would help with the intuition that it just returns another function that is then called, so it's necessary to add the extra chained parenthesis with any possible needed arguments.
But of course, I can change the names if there's another preference.
Tests and Docs
I want to gather feedback first before writing tests and docs, but I'll add them if this or something like this is acceptable.
Note on FastAPI
I want to have this type of interface for users and myself, I use autocompletion and tooling/types a lot. I thought of adding something like this to FastAPI itself, but I think this would really belong here, not in FastAPI.
I also intend to add it to the FastAPI docs to explain "advanced" use cases with concurrency and blocking code in threads run from async code, etc. I would like to document all those things in FastAPI referring to and explaining AnyIO directly, instead of writing wrappers in FastAPI or anything else. 🤓
2022-01-08 Edit
I see this (in particular taking kwargs) is quite debated, even more in Trio. And I know this is a somewhat drastic change, so I built a small package (Asyncer) just with these functions and a couple of extra things. I documented it as temporary (in case these things land here) and very subjective (to my point of view).
Maybe that could help gauge how useful that API style is for Trio and AnyIO before making any commitments, in a safer way. I don't like the feel of "yet another library", but it might help separate what is more stable (Trio and AnyIO) and what is a Frankenstein temporary experiment (this idea(s) I'm proposing). I'm mentioning it here just for completeness, just in case.