Skip to content

Fix TOPP parameter handling for zero values#328

Merged
t0mdavid-m merged 1 commit intomainfrom
fix_parameter_handling
Jan 29, 2026
Merged

Fix TOPP parameter handling for zero values#328
t0mdavid-m merged 1 commit intomainfrom
fix_parameter_handling

Conversation

@t0mdavid-m
Copy link
Member

@t0mdavid-m t0mdavid-m commented Jan 29, 2026

Changed truthiness check if v: to explicit if v != "" and v is not None: to properly handle numeric zero values (0, 0.0) which are valid parameters.

Previously, setting fragment_bin_offset=0 would skip the value entirely, causing malformed command lines where the next flag was interpreted as the previous parameter's value.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed parameter handling to correctly process numeric zero values (0, 0.0) in command execution, while still properly filtering empty strings and None values.

✏️ Tip: You can customize this high-level summary in your review settings.

Changed truthiness check `if v:` to explicit `if v != "" and v is not None:`
to properly handle numeric zero values (0, 0.0) which are valid parameters.

Previously, setting fragment_bin_offset=0 would skip the value entirely,
causing malformed command lines where the next flag was interpreted as
the previous parameter's value.
@coderabbitai
Copy link

coderabbitai bot commented Jan 29, 2026

📝 Walkthrough

Walkthrough

Modified parameter value filtering logic in CommandExecutor.py to use explicit checks for empty strings and None instead of generic truthiness evaluation. This allows numeric zero values (0, 0.0) to be included in TOPP command construction while still excluding empty strings and None values.

Changes

Cohort / File(s) Summary
Parameter Value Filtering
src/workflow/CommandExecutor.py
Updated conditional logic for skipping parameter values during command construction. Changed from falsy checks (not value) to explicit checks (value == "" or value is None), enabling numeric zeros to pass through while maintaining exclusion of empty strings and None.

Possibly related PRs

Poem

🐰 Zero's not hollow when placed in a command,
No more wrongfully shunned by our truthiness sand!
Where once naught but emptiness passed the strict gate,
Now zeros shine bright—their numeric fate. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically identifies the main change: fixing parameter handling to accept zero values instead of skipping them.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@t0mdavid-m t0mdavid-m merged commit ae20b2c into main Jan 29, 2026
5 of 7 checks passed
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.

1 participant