Repository navigation
Fix ParseKwargs crash on a --model-kwargs value containing '=' or a non-literal - #2758
Merged
rwightman merged 2 commits intoSep 1, 2026
Merged
Conversation
…on-literal
value.split('=') split on every '=', so a value with an '=' in it (e.g.
note=size=large, or a URL with a query string) raised 'too many values to
unpack'. Split on the first '=' only. The literal_eval fallback also only
caught ValueError, so a value that is not a valid Python literal (invalid
syntax like 'size=large') raised SyntaxError instead of falling back to a
string; catch SyntaxError too.
Collaborator
|
@winklemad there was an existing test_utils.py that this PR stomps over... |
…lacing it The branch predated the current tests/test_utils.py; rebased so the ParseKwargs tests are added alongside the existing freeze/unfreeze and AverageMeter tests rather than overwriting them.
Contributor
Author
|
Good catch, sorry about that @rwightman — the branch predated the current |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
ParseKwargs(used by--model-kwargs/--opt-kwargsin the train/validate scripts) crashes on some valid inputs:Two issues in the same spot:
key, value = value.split('=')splits on every=, so any value containing an=(e.g.note=size=large, or a URL with a query string) raises too many values to unpack.literal_evalfallback only catchesValueError. A value that isn't a valid Python literal but is invalid syntax (likesize=large) raisesSyntaxError, which isn't caught, so it crashes instead of falling back to a string.Fix
=only (split('=', 1)).SyntaxErroras well asValueErrorin the string fallback.Both are minimal and the only
key=valueparser in the tree (grepconfirmsmisc.py:29is the sole.split('=')).After
Existing literal parsing (ints, floats, bools, dicts) is unchanged.
Tests
Added
tests/test_utils.pycovering literal values, a value containing=, and the non-literal string fallback. Thenote=size=largecase fails onmain(both bugs) and passes here.