Skip to content

[BugFix]: Fix local mypy issues#34739

Merged
vllm-bot merged 4 commits intovllm-project:mainfrom
hickeyma:fix-pre-commit
Feb 23, 2026
Merged

[BugFix]: Fix local mypy issues#34739
vllm-bot merged 4 commits intovllm-project:mainfrom
hickeyma:fix-pre-commit

Conversation

@hickeyma
Copy link
Contributor

@hickeyma hickeyma commented Feb 17, 2026

Purpose

When you run pre-commit in local dev environment, you get the following mypy errors:

$ pre-commit run --all-files    

ruff check..........................................................................................Passed
ruff format.........................................................................................Passed
typos...............................................................................................Passed
clang-format........................................................................................Passed
Lint GitHub Actions workflow files..................................................................Passed
pip-compile.........................................................................................Passed
reformat nightly_torch_test.txt to be in sync with test.in..........................................Passed

Run mypy locally for lowest supported Python version................................................Failed
- hook id: mypy-local
- exit code: 1

$ mypy --python-version 3.10 --follow-imports skip 
vllm/entrypoints/openai/chat_completion/protocol.py:466: error: Value of type variable "_DataclassT" of "replace" cannot be "StructuredOutputsParams"  [type-var]
vllm/entrypoints/openai/responses/protocol.py:339: error: Unexpected keyword argument "json" for "StructuredOutputsParams"  [call-arg]
vllm/entrypoints/openai/completion/protocol.py:272: error: Value of type variable "_DataclassT" of "replace" cannot be "StructuredOutputsParams"  [type-var]
vllm/distributed/kv_transfer/kv_connector/v1/nixl_connector.py:938: error: Module has no attribute "sched_setaffinity"  [attr-defined]
vllm/entrypoints/openai/responses/serving.py:503: error: Value of type variable "_DataclassT" of "replace" cannot be "StructuredOutputsParams"  [type-var]
Found 5 errors in 5 files (checked 645 source files)

[...]

This PR fixes the issues as shown below.

MyPy issues handled as follows:

  • Pydantic dataclasses does not satisfy mypy's type system with replace(), even though they are fully compatible at runtime. Therefore, ignore
  • Skipped import issue that StructuredOutputsParams has with json prioperty in vllm/entrypoints/openai/responses/protocol.py. Therefore, ignore as enabling the imports raises lot of other issues which are unrelated.
  • sched_setaffinity() is a Linux only OS command and therefore added a check in llm/distributed/kv_transfer/kv_connector/v1/nixl_connector.py for it.

Additionally:

  • Renamed json import because property called json in class also in vllm/sampling_params.py. This is for hygiene reasons.

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

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 addresses several mypy issues found during local development. The changes primarily involve adding type: ignore comments to suppress type errors related to Pydantic dataclasses and dataclasses.replace, which is a reasonable approach for these known mypy limitations. A platform-specific check for os.sched_setaffinity has been correctly added to prevent errors on non-Linux systems. However, there's a potential issue in vllm/sampling_params.py where import json_mod is used. This seems to assume a new file that isn't part of the PR. I've suggested using a standard alias import json as json_mod to resolve the name clash cleanly.

@njhill
Copy link
Member

njhill commented Feb 18, 2026

Thanks @hickeyma, I haven't looked closely but I think this may be a dup of #34693?

@hickeyma
Copy link
Contributor Author

Thanks @hickeyma, I haven't looked closely but I think this may be a dup of #34693?

Thanks @njhill for making me aware of it, I didn't see it when I checked before I started my PR. Yes, there is duplicate for StructuredOutputsParams replace() issue but not for the other fixes. Also, I took a different approach to StructuredOutputsParams replace() issue as commented in #34693 (review).

Copy link
Member

@hmellor hmellor left a comment

Choose a reason for hiding this comment

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

As mentioned in the other PR, you could try

vllm/vllm/config/utils.py

Lines 105 to 113 in 1391378

def replace(dataclass_instance: ConfigT, /, **kwargs) -> ConfigT:
"""Like [`dataclasses.replace`](https://docs.python.org/3/library/dataclasses.html#dataclasses.replace),
but compatible with Pydantic dataclasses which use `pydantic.fields.Field` instead
of `dataclasses.field`"""
cls = type(dataclass_instance)
dataclass_dict = dataclass_instance.__dict__
dataclass_dict = {k: v for k, v in dataclass_dict.items() if is_init_field(cls, k)}
dataclass_dict.update(kwargs)
return cls(**dataclass_dict)

which is pydntic compatible

@hickeyma
Copy link
Contributor Author

hickeyma commented Feb 19, 2026

Thanks @hmellor for pointing out an existing implementation of replace(). What is the value of adding the override method? If its just for mypy type error, if so is it worth the overhead and potential maintenance?

@hickeyma
Copy link
Contributor Author

Ok, I think I see. You lose the validation otherwise.

@hickeyma hickeyma requested a review from hmellor February 19, 2026 11:57
@hickeyma
Copy link
Contributor Author

@hmellor I have updated to use the pydantic compatible replace() function.

@hmellor
Copy link
Member

hmellor commented Feb 19, 2026

The main reason that dataclasses.replace isn't compatible with pydantic dataclasses is that it doesn't look inside pydantic.fields.Field to check for init. So then it will try to initialise the dataclass with fields that are post-init-only. This reimplementation respects the init by checking is_init_field.

And yes it allows us to continue using pydantic and all the validation that comes with it

hickeyma added a commit to hickeyma/vllm that referenced this pull request Feb 19, 2026
Review comment:
- vllm-project#34739 (comment)

Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
@hickeyma hickeyma requested a review from hmellor February 19, 2026 16:58
@hickeyma hickeyma requested a review from hmellor February 20, 2026 11:12
@mergify
Copy link

mergify bot commented Feb 20, 2026

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

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 Feb 20, 2026
MyPy issues handled as follows:

- Pydantic dataclasses do not satisfy mypy's type system with replace(),
even though they are fully compatible at runtime. Therefore, ignore
- Skipped import issue StructuredOutputsParams in
vllm/entrypoints/openai/responses/protocol.py. Therefore, ignore.
- sched_setaffinity is Linux only OS command and therefore added check
in llm/distributed/kv_transfer/kv_connector/v1/nixl_connector.py for it.

Additionally:
- Renamed json import because property called json in class also. This is
 for hygiene reasons.

Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
Replace function from vllm/vllm/config/utils.py

Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
Review comment:
- vllm-project#34739 (comment)

Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
Copy link
Member

@hmellor hmellor left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for helping reduce the mypy based weirdness in vLLM!

@hmellor hmellor enabled auto-merge (squash) February 20, 2026 16:45
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Feb 20, 2026
@hickeyma
Copy link
Contributor Author

LGTM, thanks for helping reduce the mypy based weirdness in vLLM!

Thanks @hmellor for the reviews and feedback.

@vllm-bot vllm-bot merged commit e97c46a into vllm-project:main Feb 23, 2026
54 of 59 checks passed
@hickeyma hickeyma deleted the fix-pre-commit branch February 23, 2026 08:42
llsj14 pushed a commit to llsj14/vllm that referenced this pull request Mar 1, 2026
Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
Co-authored-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
tunglinwood pushed a commit to tunglinwood/vllm that referenced this pull request Mar 4, 2026
Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
Co-authored-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
askliar pushed a commit to askliar/vllm that referenced this pull request Mar 9, 2026
Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
Co-authored-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Andrii Skliar <askliar@nvidia.com>
Copilot AI pushed a commit to machov/vllm that referenced this pull request Mar 10, 2026
Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
Co-authored-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working frontend kv-connector 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