Skip to content

Preserve equals signs in CLI keyword values - #1180

Merged
BenjaminBossan merged 2 commits into
skorch-dev:masterfrom
laulauraluo:fix/cli-keyword-equals
Oct 26, 2026
Merged

BenjaminBossan merged 2 commits into
skorch-dev:masterfrom
laulauraluo:fix/cli-keyword-equals

Conversation

@laulauraluo

Copy link
Copy Markdown
Contributor

A CLI object argument such as builtins.dict(label="epoch=1") raises ValueError because keyword parsing splits on every =. Split only on the first delimiter so the rest of the string stays in the value.

Added regression coverage for strings with one or multiple equals signs, alongside a plain-string control. The two regression cases fail before the fix; pytest skorch/tests/test_cli.py passes all 45 tests after it.

@BenjaminBossan BenjaminBossan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this fix, LGTM. Just a comment about the test.

Comment thread skorch/tests/test_cli.py Outdated
assert np.isclose(parsed_kwargs['module__nonlin'].upper, 0.456)

@pytest.mark.parametrize('label', ['epoch', 'epoch=1', 'epoch=1=best'])
def test_parse_net_kwargs_string_with_equals(self, parse_net_kwargs, label):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This case could be easily integrated into the test above, right?

@BenjaminBossan BenjaminBossan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for fixing, LGTM.

@BenjaminBossan
BenjaminBossan merged commit 066927b into skorch-dev:master Oct 26, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants