Turn @config into a dataclass_transform#31541
Turn @config into a dataclass_transform#31541DarkLight1337 merged 20 commits intovllm-project:mainfrom
@config into a dataclass_transform#31541Conversation
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>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request correctly refactors the @config decorator into a dataclass_transform, simplifying config class definitions by removing the need for an explicit @dataclass decorator. The changes are applied consistently across the codebase, and the tests and pre-commit hooks are updated accordingly.
I found one issue in the updated validate_config.py pre-commit hook where the logic to validate decorators is flawed. I've provided a suggestion to fix this. Other than that, the changes look good and improve the developer experience when defining new configuration classes.
|
Hi @hmellor, 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: 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>
|
This pull request has merge conflicts that must be resolved before it can be |
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>
|
Hi @hmellor, 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: 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: felix01.yu <felix01.yu@vipshop.com>
Main changes:
configdecorator into adataclass_transformconfiginstead ofconfiganddataclassConfigDictfor this transformed dataclass to forbid extra fields (cc @mgoin who added this to some configs in/vllm/config/compilation.py)Other changes:
configalonemypypre-commit hook updated because mypy needs to importconfigto understand itconfigaloneIssues that doing this revealed:
decoding_configwhich was ignored prior to this PRdictpassed toSpeculativeConfig(**kwargs)andSpeculatorsConfig(**kwargs). This was not a problem forSpeculativeConfigbecause thePretrainedConfigwould just accept the extra kwargs and ignore them.SpeculativeConfigalso used to accept and ignore these fields but by addingextra="forbid", this became a problem. This PR also untangles the generation ofkwargsfor constructing these two classes for Speculators models.