Fix AnchorText metadata including invalid parameters (#459)#1063
Open
rodriguescarson wants to merge 1 commit into
Open
Fix AnchorText metadata including invalid parameters (#459)#1063rodriguescarson wants to merge 1 commit into
rodriguescarson wants to merge 1 commit into
Conversation
AnchorText.meta['params'] was updated with all_opts, which _validate_kwargs populated from the raw kwargs and therefore included misspelled/invalid parameter names. Record only the validated perturbation parameters (self.perturb_opts) instead, and drop the now-unused all_opts from _validate_kwargs (its sole caller no longer needs it). Adds a regression test asserting invalid kwargs are excluded from meta['params'] while valid perturbation params are retained. Fixes SeldonIO#459
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.
Fixes #459
Problem
AnchorText.meta['params']recorded all parameters passed viakwargs, including misspelled or otherwise invalid ones._validate_kwargsreturned two dicts —perturb_opts(only the validated perturbation params) andall_opts(the defaults plus the rawkwargs) — and__init__updated the metadata fromall_opts:So an invalid key like
sampl_proba=0.6(typo) would be logged as incorrect but still leak intometa['params'].Fix
Per the issue, record only the validated
self.perturb_optsin the metadata, and dropall_optsentirely —_validate_kwargsis only called here, so it now returns justperturb_opts:self.meta['params'].update(**self.perturb_opts)_validate_kwargsreturn typeTuple[dict, dict]→dict; removed theall_optsdeepcopy/update.Test
Added
test_metadata_excludes_invalid_params(mirrorstest_neighbors, using thenlpfixture): constructs anAnchorTextwith one valid (sample_proba) and one invalid (not_a_real_param) kwarg and asserts the invalid one is absent frommeta['params']while the valid one is retained.I wasn't able to run the full test suite locally (it needs the spaCy model + heavy deps), but both files parse cleanly and the change matches the fix described in the issue.
🤖 Generated with Claude Code