Repository navigation
fix: make CLI messages name flags that exist and causes users can act on - #77
Merged
Merged
Conversation
Four hints pointed at flags that were renamed (or never existed):
"Subreg Mask already exists. Set -override_subreg to create it anew" # --override-semantic/-os
"Vert Mask already exists. Set -override_vert to create it anew" # --override-instance/-oi
"Centroids already exists ... Set -override_ctd = True" # --override-ctd/-oc
"-model_instance/-mv resolved to None" # --model-instance/-mi
and `--input` mishandled the two most common mistakes. An uncompressed `scan.nii`, which
SPINEPS cannot read at all, had `.nii.gz` appended unconditionally and then failed with
"-input does not exist or is not a file, got /abs/scan.nii.nii.gz" -- a path the user
never typed. It now says the image must be gzipped and how to fix it, while
`--input sub-01_T2w` (no extension) keeps working because the suffix is only appended
when nothing exists at the given path. A dead `if dataset == ""` branch (unreachable
after `Path.absolute()`) is removed.
Also stops the log-argument filter from logging its own exclusion list.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`--model-{kind}` is assembled at runtime, so the literal `--model` prefix is not a flag.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`--model-{kind}` is assembled at runtime, so `--model` is not a flag the parser has to know.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A hardcoded POSIX path never appears in the message on Windows, where the input is absolutised to D:\this\path\... Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Problem
Hints naming flags that do not exist. All four survived the 2.0 flag rename:
Subreg Mask already exists. Set -override_subreg to create it anew-override_subreg--override-semantic/-osVert Mask already exists. Set -override_vert to create it anew-override_vert--override-instance/-oiCentroids already exists, will load instead. Set -override_ctd = True-override_ctd--override-ctd/-oc-model_instance/-mv resolved to None-model_instance--model-instance/-miA user who follows the hint gets
error: unrecognized arguments.--inputon an uncompressed NIfTI..niiis what most converters produce, and SPINEPS cannot readit (
BIDS_FILEkeys the file asnii, the pipeline asks fornii.gz). The old code appended the suffixunconditionally:
That names a path the user never typed and says nothing about compression.
Fix
--inputvalidation, in order: append.nii.gzonly if nothing exists at the path as given (so--input sub-01_T2wkeeps working), then report a missing folder, then a missing file, then -- for afile that is there but is not
.nii.gz-- "SPINEPS reads gzipped NIfTI images (.nii.gz), got scan.nii.Compress it first, e.g. 'gzip scan.nii'."
if dataset == ""branch, unreachable since the path is passed throughPath.absolute().process_dataset's logged arguments no longer include the name of the exclusion list itself.Tests
unit_tests/test_cli_messages.pyincludes a guard that walks every user-facing string inentrypointand
seg_run, extracts anything shaped like a flag, and asserts the parser accepts it -- so the nextrename cannot leave a stale hint behind. It catches all four on
main, plus the.niimessage.Full suite: 229 passed, ruff clean.
🤖 Generated with Claude Code