[mypy] Enable following imports for some directories #6681
Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. Once the PR is approved and ready to go, please make sure to run full CI as it is required to merge (or just use auto-merge). To run full CI, you can do one of these:
🚀 |
DarkLight1337
added
the
ready
DarkLight1337
force-pushed
the
mypy-follow-imports
branch
from
9833350
to
3696f63
Compare
| mypy vllm/transformers_utils --config-file pyproject.toml | ||
| mypy vllm/usage --config-file pyproject.toml | ||
| mypy vllm/worker --config-file pyproject.toml | ||
| mypy tests --follow-imports skip |
There was a problem hiding this comment.
the new formatter and github action doesn't pass --config-file, is this intended?
There was a problem hiding this comment.
Yes. The TOML file is already being used by default so there is no need to specify it explicitly.
| @@ -94,8 +94,10 @@ def __contains__(self, key: Hashable) -> bool: | |||
| def __len__(self) -> int: | |||
| return len(self.cache) | |||
|
|
|||
| def __getitem__(self, key: Hashable) -> Optional[T]: | |||
| return self.get(key) | |||
| def __getitem__(self, key: Hashable) -> T: | |||
There was a problem hiding this comment.
is this related to this PR?
| @@ -109,8 +111,9 @@ def touch(self, key: Hashable) -> None: | |||
| def get(self, | |||
| key: Hashable, | |||
| default_value: Optional[T] = None) -> Optional[T]: | |||
| value: Optional[T] | |||
There was a problem hiding this comment.
this doesn't seem to be optional anymore this way?
There was a problem hiding this comment.
(iiuc if there's no item, it should raise an exception?)
There was a problem hiding this comment.
Yes, that is the intention so it works more like a Python dictionary. This is required to fix some errors in type annotation where it's obviously expected that the value exists in the cache, such as
The
follow_importsoption has been enabled for the following files and directories:vllm/*.py(originally failedmypy, made passing in this PR)vllm/adapter_commons(originally failedmypy, made passing in this PR)vllm/assetsvllm/inputsvllm/loggingvllm/multimodalvllm/platformsvllm/servervllm/transformers_utils(originally failedmypy, made passing in this PR)vllm/triton_utils(originally failedmypy, made passing in this PR)vllm/usageFuture code modifying those files and directories will thus be subject to stricter type checks - another step towards fulfilling #3680.
Note that the config in
pyproject.tomlhas been updated such thatfollow_importsis now enabled by default, but only checks the above directories. After updating another directory to pass withfollow_importsenabled, please add a corresponding entry inpyproject.tomlthen in turn remove it fromformat.shandmypy.yaml.