Skip to content

[CI] Fix Pre-commit Issue Cannot determine type of "rank" and "world_size"#26448

Merged
vllm-bot merged 7 commits intomainfrom
wentao-fix-pre-commit-issue
Oct 9, 2025
Merged

[CI] Fix Pre-commit Issue Cannot determine type of "rank" and "world_size"#26448
vllm-bot merged 7 commits intomainfrom
wentao-fix-pre-commit-issue

Conversation

@yewentao256
Copy link
Copy Markdown
Member

@yewentao256 yewentao256 commented Oct 8, 2025

Purpose

Fix pre-commit issue

vllm/distributed/device_communicators/all2all.py:393: error: Cannot determine type of "rank"  [has-type]
vllm/distributed/device_communicators/all2all.py:394: error: Cannot determine type of "world_size"  [has-type]

Signed-off-by: yewentao256 <zhyanwentao@126.com>
@yewentao256 yewentao256 added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 8, 2025
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request fixes a mypy type checking error by adding redundant type annotations. While this is an effective workaround, it's important to address the underlying issue to improve code maintainability. My review includes a comment on this matter.

@mgoin
Copy link
Copy Markdown
Member

mgoin commented Oct 8, 2025

@yewentao256 where is this precommit issue? Latest commits on main are passing fine

@yewentao256
Copy link
Copy Markdown
Member Author

yewentao256 commented Oct 8, 2025

@yewentao256 where is this precommit issue? Latest commits on main are passing fine

@mgoin If you try to upload something diff with that file, this pre-commit issue will trigger locally

@hmellor
Copy link
Copy Markdown
Member

hmellor commented Oct 9, 2025

Could you instead try moving vllm/distributed from SEPARATE_GROUPS to FILES:

FILES = [
"vllm/*.py",
"vllm/assets",
"vllm/entrypoints",
"vllm/inputs",
"vllm/logging_utils",
"vllm/multimodal",
"vllm/platforms",
"vllm/transformers_utils",
"vllm/triton_utils",
"vllm/usage",
]
# After fixing errors resulting from changing follow_imports
# from "skip" to "silent", move the following directories to FILES
SEPARATE_GROUPS = [
"tests",
"vllm/attention",
"vllm/compilation",
"vllm/distributed",
"vllm/engine",
"vllm/executor",
"vllm/inputs",
"vllm/lora",
"vllm/model_executor",
"vllm/plugins",
"vllm/worker",
"vllm/v1",
]

There will probably be some other mypy errors but this will be better for the long term health of vLLM

Signed-off-by: yewentao256 <zhyanwentao@126.com>
@yewentao256 yewentao256 requested a review from hmellor as a code owner October 9, 2025 20:46
Signed-off-by: yewentao256 <zhyanwentao@126.com>
@yewentao256
Copy link
Copy Markdown
Member Author

yewentao256 commented Oct 9, 2025

@hmellor Thanks! There would be a lot of changes needed to solve the mypy issues within that folder, could we can make it in following-up issues/PRs?

#26533

@hmellor
Copy link
Copy Markdown
Member

hmellor commented Oct 9, 2025

Ok I just tried and saw 47 issues. Many of them are likely linked to a few configs being marked as optional when they shouldn't be, so it likely won't require 47 changes.

But yes this can be follow up work. Please just add a comment saying that the duplicate type hinting can be removed once #26533 is closed?

Signed-off-by: yewentao256 <zhyanwentao@126.com>
@yewentao256
Copy link
Copy Markdown
Member Author

@hmellor Done, thanks!

@hmellor hmellor enabled auto-merge (squash) October 9, 2025 21:25
@vllm-bot vllm-bot merged commit 8983e02 into main Oct 9, 2025
42 of 55 checks passed
@vllm-bot vllm-bot deleted the wentao-fix-pre-commit-issue branch October 9, 2025 22:16
Dhruvilbhatt pushed a commit to Dhruvilbhatt/vllm that referenced this pull request Oct 14, 2025
…size" (vllm-project#26448)

Signed-off-by: yewentao256 <zhyanwentao@126.com>
Signed-off-by: Dhruvil Bhatt <bhattdbh@amazon.com>
bbartels pushed a commit to bbartels/vllm that referenced this pull request Oct 16, 2025
…size" (vllm-project#26448)

Signed-off-by: yewentao256 <zhyanwentao@126.com>
Signed-off-by: bbartels <benjamin@bartels.dev>
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
…size" (vllm-project#26448)

Signed-off-by: yewentao256 <zhyanwentao@126.com>
alhridoy pushed a commit to alhridoy/vllm that referenced this pull request Oct 24, 2025
…size" (vllm-project#26448)

Signed-off-by: yewentao256 <zhyanwentao@126.com>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
…size" (vllm-project#26448)

Signed-off-by: yewentao256 <zhyanwentao@126.com>
Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
…size" (vllm-project#26448)

Signed-off-by: yewentao256 <zhyanwentao@126.com>
Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
…size" (vllm-project#26448)

Signed-off-by: yewentao256 <zhyanwentao@126.com>
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
…size" (vllm-project#26448)

Signed-off-by: yewentao256 <zhyanwentao@126.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants