Skip to content

rabbit_plot_cov: name the missing --params entry instead of raising TypeError - #227

Merged
davidwalter2 merged 1 commit into
WMass:mainfrom
pmlugato:plot-cov-missing-param
Nov 9, 2026
Merged

davidwalter2 merged 1 commit into
WMass:mainfrom
pmlugato:plot-cov-missing-param

Conversation

@pmlugato

@pmlugato pmlugato commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

args.params is a list. On the error path for a parameter missing from the fit result, args.params[~isin] raised TypeError, which hid the missing name. The list is now converted with np.asarray first, so the RuntimeError names the parameter. Valid calls are unchanged.

🤖 Generated with Claude Code

…ypeError

args.params is a list, so boolean-mask indexing it on the error path raised
TypeError and hid which parameter was not in the fit result.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issues Found

No issues found.

Summary

Total issues: 0 critical, 0 important, 0 minor.

Overall Verdict

Status: Patch is correct

Explanation: The fix correctly converts the argparse list args.params to a numpy array before boolean-mask indexing, so the error path now raises the intended RuntimeError naming the missing parameter instead of a TypeError. Semantics for valid calls are unchanged (np.isin converts lists identically), and subsequent uses of args.params (e.g., the indices loop) still operate on the original list as before.

Review details
  • Model: cern/Kimi-K3-FLATIRON

Review metrics: 3 turns · 4 tool calls · 55s

  • Tokens: 14.1K input · 9.7K cached (69%) · 2.8K output

@cmsmwbot cmsmwbot Bot added the fit label Nov 9, 2026
@cmsmwbot

cmsmwbot Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

CI status — workflow Hodor AI review, run 42661716674: ✅ 1/1 jobs green (slowest/only review, 1m22s). The main CI workflow for head 382faed8 is still in progress — one consolidated note when it finishes.

@cmsmwbot

cmsmwbot Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

CI green ✅ — 19/19 jobs successful on 382faed (run 42661717175). Slowest: all-unit-tests 1h21m. Ready for review.

@davidwalter2
davidwalter2 merged commit 2f115a9 into WMass:main Nov 9, 2026
20 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