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

Incorrect type annotation on chain_future method #3314

Closed
arnaudsjs opened this issue Aug 16, 2023 · 0 comments · Fixed by #3393
Closed

Incorrect type annotation on chain_future method #3314

arnaudsjs opened this issue Aug 16, 2023 · 0 comments · Fixed by #3393

Comments

@arnaudsjs
Copy link

Problem description:

The arguments of the tornado.concurrent.chain_future method are annotated with the asyncio.Future type. The documentation and implementation of this method on the other hand indicates it also supports concurrent.futures.Future instances. This way mypy raises a type error when a concurrent.futures.Future instances is used.

Reproductive example:

# test/__init__.py
from concurrent.futures import Future
from tornado import concurrent as tornado_concurrent

f1: Future = Future()
f2: Future = Future()
tornado_concurrent.chain_future(f1, f2)

Mypy output:

$ mypy -p test

test/__init__.py:7: error: Argument 1 to "chain_future" has incompatible type "concurrent.futures._base.Future[Any]"; expected "asyncio.futures.Future[<nothing>]"  [arg-type]
test/__init__.py:7: error: Argument 2 to "chain_future" has incompatible type "concurrent.futures._base.Future[Any]"; expected "asyncio.futures.Future[<nothing>]"  [arg-type]
Found 2 errors in 1 file (checked 1 source file)

Expected behavior:

The chain_future() method has a type annotation that also accepts concurrent.futures.Future instances and mypy doesn't raise a type error.

bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 7, 2024
This method has always accepted both asyncio and concurrent futures,
but the type hint incorrectly indicated that it only accepted asyncio
futures.

Fixes tornadoweb#3314
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 a pull request may close this issue.

1 participant