Skip to content

save_script_parameters: skip whole names only - #162

Open
CellKai wants to merge 1 commit into
imcf:develfrom
CellKai:fix-save-script-parameters-skip
Open

CellKai wants to merge 1 commit into
imcf:develfrom
CellKai:fix-save-script-parameters-skip

Conversation

@CellKai

@CellKai CellKai commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Branch fix-save-script-parameters-skip, based on devel (8482d1a).

imcflibs.imagej.misc.save_script_parameters matched its skip list (USERNAME, SJLOG, COMMAND,
RM) as substrings of the parameter names, so ordinary parameters containing one of them were
silently left out of the saved file, e.g. fusion_format (contains RM), format, normalize,
firmware, informed, ... The docstring describes the list as runtime keys to skip (case
insensitive), i.e. whole names.

Fix: if key.upper() in skip_keys. New test test_save_script_parameters_skip_whole_names
(rm skipped, fusion_format and commander kept); it fails before the fix. All tests pass,
ruff check and markdownlint clean.

The skip list ("USERNAME", "SJLOG", "COMMAND", "RM") was matched as a
substring, so e.g. a parameter "fusion_format" (contains "RM") was not
saved. Match the whole name (case insensitive), as documented.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 28, 2026 16:54

Copilot AI 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.

🟢 Approval recommended

No active review issues are reported for the parameter-name matching fix.

No open issues 🎉

What changed in this PR

Fixes save_script_parameters so the case-insensitive skip list matches whole parameter names, preventing ordinary parameters such as fusion_format from being omitted.

Changes:

  • Replaces substring matching with exact-name membership.
  • Adds a regression test confirming rm is skipped while fusion_format and commander are saved.
  • Documents the corrected behavior in the changelog.
File Description
tests/​test_misc.py Adds regression coverage for whole-name, case-insensitive skipping.
src/​imcflibs/​imagej/​misc.py Uses exact-name matching and clarifies the explanatory comment.
CHANGELOG.md Records the parameter-saving fix.

Review effort: Balanced

@codecov

codecov Bot commented Oct 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 33%. Comparing base (8482d1a) to head (bf5b534).

Additional details and impacted files
@@         Coverage Diff          @@
##           devel   #162   +/-   ##
====================================
  Coverage     33%    33%           
====================================
  Files         25     25           
  Lines       1806   1806           
====================================
  Hits         591    591           
  Misses      1215   1215           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ehrenfeu
ehrenfeu self-requested a review October 28, 2026 18:26

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants