Proposal: force type hint check with mypy

[wangkuiyi]

Why

While I am learning the source code, I found it challenging to understand asyncio. Taking this codebase as an example, I spend quite some time with the following method signature:

vllm/vllm/engine/async_llm_engine.py

Lines 394 to 399 in 7c60044

	async def generate(
	self,
	prompt: Optional[str],
	sampling_params: SamplingParams,
	request_id: str,
	prompt_token_ids: Optional[List[int]] = None) -> RequestOutput:

I realized there was a discrepancy in the return type hint. The original hint was:

-> RequestOutput:

However, the correct hint should be:

-> typing.AsyncIterator[RequestOutput]:

For a detailed demonstration of this issue, please see the appendix below.

Proposal

To enhance type checking with mypy, it's advisable to include the following options in mypy.ini, as suggested in this StackOverflow answer:

disallow_untyped_defs
disallow_incomplete_defs
disallow_untyped_calls

Appendix

Below is a complete example that illustrates the correct use of type hints for async iterators:

"""
For a full check including type hints, run the following command

mypy \
  --disallow-untyped-defs \
  --disallow-incomplete-defs \
  --disallow-untyped-calls \
  /tmp/a.py
"""
import asyncio
import typing


async def foo() -> typing.AsyncIterable[int]:
    for i in range(3):
        yield (i)


async def bar() -> typing.AsyncIterable[int]:
    f = foo()
    print(f"The type of foo is {type(foo)}")  # <class 'function'>
    print(f"The return type of foo is {type(f)}")  # <class 'async_generator'>
    async for i in f:
        yield i


async def main() -> None:
    b = bar()
    print(f"The type of bar is {type(bar)}")  # <class 'function'>
    print(f"The return type of bar is {type(b)}")  # <class 'async_generator'>
    async for i in b:
        print(i)


c = main()
print(f"The return type of main is {type(c)}")  #  <class 'coroutine'>
asyncio.run(c)

[simon-mo]

we would definitely love to fully enable type checking, however, we understand it is fairly involved and time consuming so we are focusing on more pressing issue. if you can help contribute that would be amazing!

[DarkLight1337]

@rkooo567 you may close this once you've completed the PR for cross-directory type checking with mypy.

[rkooo567]

Yeah I will need to create 1 more PR to finish this

[github-actions]

This issue has been automatically marked as stale because it has not had any activity within 90 days. It will be automatically closed if no further activity occurs within 30 days. Leave a comment if you feel this issue should remain open. Thank you!

[DarkLight1337]

Closing this as mypy has been enabled to some extent - please refer to #3680 for further developments.