fix: WeakspotsDiagnosis crash on custom metrics subset#541
Conversation
When a custom metrics= dict is a strict subset of the default metric names and no thresholds= is supplied, pass_thresholds still carried all four default keys while pass_columns narrowed to the subset, so the df[pass_columns].lt(pass_thresholds) comparison raised "Unable to coerce to Series, length must be 1: given 4". Narrow the thresholds to pass_columns before the comparison; plot_thresholds is left untouched so charts keep reference lines for all metrics. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PR SummaryThis PR introduces several enhancements to the WeakspotsDiagnosis component by refining its handling of custom metrics and threshold comparisons. The changes are twofold:
Overall, these changes increase the flexibility in defining and evaluating metric thresholds, making the component more robust when using customized scoring logic. Test Suggestions
|
|
Nice fix — I traced the pass/fail path end to end and it holds up. Narrowing the threshold dict to The tests are the part I liked most — they don't just prove the crash is gone, they engineer the data so the default run fails on Accuracy while the F1-only run passes, which actually proves the pass/fail is scoped to the requested metric and not just silently suppressed. I re-checked that against the default thresholds and it lines up. Two small, non-blocking observations, both pre-existing rather than anything this PR introduced:
Approving — thanks for the thorough test. |
Pull Request Description
What and why?
WeakspotsDiagnosisraisesValueError: Unable to coerce to Series, length must be 1: given 4when called with a custommetrics=dict that's a strict subset of the default metric names (accuracy/precision/recall/f1) and no explicitthresholds=.The pass/fail check narrows
pass_columnsto the metrics actually requested, but compares againstpass_thresholds, which still carries all four default keys:DataFrame.lt(dict)requires the dict to match the DataFrame's column count in this usage, so a 1-column frame against a 4-key dict raises. Minimal standalone repro:This is a plausible, unremarkable usage pattern (a caller who only cares about F1) rather than an edge case anyone would think to avoid — passing a matching
thresholds=sidesteps it, but nothing signals that requirement.Found as a side-effect of implementing sc-17261 (the ZD-704/#534 follow-up); tracked separately as sc-17267 since it's an independent bug, not tied to that customer ticket.
What changed
Narrows
pass_thresholdstopass_columnsright before the comparison.plot_thresholds(used for chart reference lines) is untouched, so charts still show reference lines for every metric — only the pass/fail check is scoped to what was actually requested.How to test
The added test builds a case where the default (all-four-metric) run fails on Accuracy while an F1-only run passes on the same data — proving the fix actually scopes pass/fail correctly rather than just suppressing the crash.
pytest tests/unit_tests/model_validation/sklearn/test_WeakspotsDiagnosis.py -q→ 15 passed.What needs special review?
The distinguishing test relies on binary data where F1 ≥ 0.7 mathematically forces precision/recall above their own 0.5 thresholds, so Accuracy is the only metric that can independently fail — worth a glance to confirm that reasoning holds.
Dependencies, breaking changes, and deployment notes
None. Default (all-metrics) and full-thresholds-supplied paths are unaffected — this only changes behavior for the previously-crashing subset-without-matching-thresholds case.
Release notes
Fixed
WeakspotsDiagnosisraisingValueError: Unable to coerce to Series, length must be 1: given 4when called with a custommetrics=subset and no matchingthresholds=.Checklist
Closes sc-17267
🤖 Generated with Claude Code