Skip to content

Conversation

hmellor
Copy link
Member

@hmellor hmellor commented Sep 20, 2025

  • Ports the bahaviour of mypy.sh to mypy.py
  • mypy.py takes the file list as input and will only run on those files
  • For some reason mypy --follow-imports skip (no directory passed) behaves slightly differently when pre-commit passes all the files it would have checked implicitly based on pyproject.toml. This is why there minor typing changes in 4 unrelated files.

Example log from the first commit in this PR:

...
Run mypy for local Python installation..............................................................Passed
- hook id: mypy-local
- duration: 0.23s

$ mypy --python-version 3.12 --follow-imports skip 
Success: no issues found in 4 source files

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Copy link
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 refactors the mypy pre-commit hook by replacing the mypy.sh shell script with a more robust Python script, mypy.py. This change improves maintainability and correctly handles file lists from pre-commit. The configuration in .pre-commit-config.yaml is also simplified using YAML anchors. My review focuses on the new Python script, where I've found a couple of critical issues with incorrect type hints that should be addressed to ensure the script's correctness.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) September 21, 2025 03:36
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 21, 2025
Copy link

mergify bot commented Sep 21, 2025

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @hmellor.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Sep 21, 2025
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
@mergify mergify bot removed the needs-rebase label Sep 22, 2025
@DarkLight1337 DarkLight1337 merged commit 3d2c56b into vllm-project:main Sep 22, 2025
43 checks passed
@hmellor hmellor deleted the make-mypy-nice branch September 22, 2025 12:24
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
charlifu pushed a commit to ROCm/vllm that referenced this pull request Sep 25, 2025
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: charlifu <charlifu@amd.com>
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
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
ci/build frontend 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.

3 participants