[Misc][BE] Type coverage for vllm/compilation [1/3]#31554
[Misc][BE] Type coverage for vllm/compilation [1/3]#31554zou3519 merged 2 commits intovllm-project:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request significantly improves the type coverage for the vllm/compilation module. The added type hints are accurate and comprehensive, covering function signatures, variables, and return types, which will enhance code maintainability, readability, and help prevent silent errors. The use of more specific types like Generator, Literal, ParamSpec, and TypeVar is particularly noteworthy. Additionally, a crucial assertion was added to prevent a potential runtime TypeError, further improving the code's robustness. The changes are well-executed and represent a valuable improvement to the codebase.
|
Hi @Lucaskabela, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
Signed-off-by: Lucas Kabela <lucaskabela@meta.com>
51066b2 to
40793b5
Compare
| def is_func(node: fx.Node, target) -> bool: | ||
| return node.op == "call_function" and node.target == target | ||
| def is_func(node: fx.Node, target: Target) -> bool: | ||
| return bool(node.op == "call_function" and node.target == target) |
There was a problem hiding this comment.
why does this need a cast to bool?
There was a problem hiding this comment.
vLLM type checking is in a kind of weird mode - the follow_imports currently is set for pre-commit (locally), which for the case of some torch functions, means they are stubbed as Any types. This raises an error mismatch (returning Any when declared type bool), so an explicit cast makes the linter happy here
There was a problem hiding this comment.
For all the directories in
follow imports has its default value for mypy. Note that vllm/compilation is not included in this list.
For all the directories in
Lines 58 to 67 in 3c3c547
mypy is run separately with --follow-imports skip.
Once all the import following related issues have been fixed for a directory in SEPARATE_GROUPS, it should be moved to FILES.
Signed-off-by: Lucas Kabela <lucaskabela@meta.com>
Signed-off-by: Lucas Kabela <lucaskabela@meta.com>
Signed-off-by: Lucas Kabela <lucaskabela@meta.com>
Signed-off-by: Lucas Kabela <lucaskabela@meta.com> Signed-off-by: dsuhinin <suhinin.dmitriy@gmail.com>
Signed-off-by: Lucas Kabela <lucaskabela@meta.com>
Purpose
We want to provide better type hint coverage in vllm/compilation to improve maintainability, readability, and reduce silent errors
Test Plan
mypy vllm/compilationEssential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.