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

✨ Add alternative APIs that improve typing and tooling support (e.g. autocompletion) #2208

Closed
wants to merge 4 commits into from

Conversation

tiangolo
Copy link
Member

@tiangolo tiangolo commented Dec 26, 2021

✨ 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:

  • Sending async tasks in a nursery (equivalent to nursery.start_soon())
  • Sending blocking tasks to a worker thread (equivalent to to_thread.run_sync())
  • Sending async tasks to the main async loop from a worker thread (equivalent to 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:

  • Equivalent to to_thread.run_sync()
  • Equivalent to from_thread.run()

Previous Discussion

I proposed this in AnyIO here: agronholm/anyio#403

I was pointed out at the discussion here: #470

That discussion revolves mainly around accepting keyword arguments for functions called, and this is also related to that. But one of the main things I want from this is better tooling and editor support: autocompletion and inline errors... not only supporting kwargs. So I thought it was worth sharing this additional perspective.

Usage

The main difference is that instead of having a function that takes the function to call plus:

  • positional arguments (and that requires using partials for keyword arguments)
  • configs (like task name)
trio_function(worker_function, arg1, arg2)

...this new API only takes the function to call plus Trio-specific 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:

trio_functionify(worker_function)(arg1, arg2)

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:

trio_functionify(worker_function)(arg1, arg2, kwarg1="a", kwarg2="b")

...but the main benefit, from my point of view, is actually better typing/tooling support. Autocompletion and inline type errors in the editor and mypy.

Examples

Nursery

import trio


async def program() -> None:
    async def worker(i: int, message: str = "Default worker"):
        print(f"{message} - worker {i}")

    async with trio.open_nursery() as nursery:
        for i in range(3):
            nursery.soonify(worker)(i=i, message="Hello ")


trio.run(program)
  • Autocompletion for a nursery:

Selection_032

  • Inline errors for a nursery:

Selection_033

  • Mypy error detection:
main.py:10: error: Too few arguments
Found 1 error in 1 file (checked 1 source file)

To Thread (asyncify)

import time
import trio


def run_slow(i: int, message: str = "Hello ") -> str:
    time.sleep(i)
    return message + str(i)


async def program() -> None:
    for i in range(3):
        result = await trio.to_thread.asyncify(run_slow)(i=i)
        result + 3
        print(result)


trio.run(program)
  • Autocompletion for sending to worker thread:

Selection_034

  • Inline errors for sending to worker thread:

Selection_035

  • Autocompletion for result value:

Selection_036

  • Inline errors for result value:

Selection_037

  • Mypy error detection:
main.py:13: error: Unsupported operand types for + ("str" and "int")
Found 1 error in 1 file (checked 1 source file)

From Thread (syncify)

import trio


async def aio_run_slow(i: int, message: str = "Async Hello ") -> str:
    await trio.sleep(i)
    return message + str(i)


def run_slow_embed(i: int) -> str:
    result = trio.from_thread.syncify(aio_run_slow)(i=i, message="Hello sincify ")
    return result


async def program() -> None:
    for i in range(3):
        result = await trio.to_thread.asyncify(run_slow_embed)(i=i)
        print(result)


trio.run(program)
  • Autocompletion for sending from worker thread:

Selection_038

  • Inline errors for sending from worker thread:

Selection_039

  • Autocompletion for result value:

Selection_040

  • Inline errors for result value:

Selection_041

  • Mypy error detection:
main.py:11: error: Unsupported operand types for + ("str" and "int")
Found 1 error in 1 file (checked 1 source file)

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.

Type Annotations

I'm also adding the bare minimum type annotation stuff to make it all work with mypy.

Note on AnyIO

I want to have this on AnyIO: agronholm/anyio#403, but AnyIO would want any decision to be taken here first, so I'm making the PR here as well, even if just as a conversation starter.

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 in Trio and AnyIO, 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 these libraries directly (AnyIO, Trio), instead of writing wrappers in FastAPI or anything else. 🤓

As I see this (in particular taking kwargs) is quite debated, I'm considering building I built a small package (Asyncer) just with those wrappers, documenting it as highly experimental and 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. 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 I'm proposing).

2022-01-08 Edit: I built Asyncer to try out this API design. 🤓

@codecov
Copy link

codecov bot commented Dec 26, 2021

Codecov Report

Merging #2208 (26ff951) into master (fc98849) will decrease coverage by 0.03%.
The diff coverage is 63.63%.

@@            Coverage Diff             @@
##           master    #2208      +/-   ##
==========================================
- Coverage   99.51%   99.47%   -0.04%     
==========================================
  Files         114      114              
  Lines       14759    14789      +30     
  Branches     2344     2346       +2     
==========================================
+ Hits        14687    14711      +24     
- Misses         47       56       +9     
+ Partials       25       22       -3     
Impacted Files Coverage Δ
trio/_threads.py 93.65% <55.55%> (-6.35%) ⬇️
trio/_core/_run.py 99.55% <69.23%> (-0.45%) ⬇️
trio/from_thread.py 100.00% <100.00%> (ø)
trio/to_thread.py 100.00% <100.00%> (ø)
trio/tests/test_ssl.py 99.58% <0.00%> (+0.55%) ⬆️
trio/_highlevel_ssl_helpers.py 100.00% <0.00%> (+11.76%) ⬆️

@Fuyukai
Copy link
Member

Fuyukai commented Dec 26, 2021

I have no real comments about the implementation of this but I am absolutely, vehemently against the concept of making libraries work around Python's anemic type system.

This should be fixed upstream, by making functools.partial properly representable, not by adding new APIs to every library that takes a callable with args.

@agronholm
Copy link
Contributor

It should also be noted that trio, for reasons beyond my understanding, does not want type annotations in the actual codebase, but instead in trio-typing.

@pquentin
Copy link
Member

pquentin commented Jan 27, 2022

It should also be noted that trio, for reasons beyond my understanding, does not want type annotations in the actual codebase, but instead in trio-typing.

This isn't true, for what it's worth. It's just that adding types after the fact is hard and the efforts to merge type hints stalled, see #1873. I'd like to add that trying to do everything in one pull request is highly likely to fail. If anyone wants to work on adding types, they should do it gradually, like urllib3 did: https://sethmlarson.dev/blog/tests-arent-enough-case-study-after-adding-types-to-urllib3

@TeamSpen210
Copy link
Contributor

Once PEP 646 (Variadic Generics) is implemented in checkers, the current APIs can be typed directly so this API change isn't as necessary.

@Zac-HD
Copy link
Member

Zac-HD commented Mar 23, 2023

Closing this since subsequent PEPs mean we'll be able to precisely type-annotate the current signatures 🙂

@Zac-HD Zac-HD closed this Mar 23, 2023
@tiangolo tiangolo deleted the alt-api-better-typing branch March 18, 2024 19:56
@rafalkrupinski
Copy link

I would love to see this merged!

@jakkdl
Copy link
Member

jakkdl commented Jun 25, 2024

We have improved the type annotations to the code now, so all of the effects of this PR should be live afaik.

@rafalkrupinski
Copy link

Can you elaborate? Documentation reads to_thread.run_sync doesn't support kwargs.

@agronholm
Copy link
Contributor

You can pass either a functools.partial() or a lambda to it.

@rafalkrupinski
Copy link

This interface would be much more convenient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants