Skip to content

ENH: Make the command-line tools usable in batch runs - #138

Open
demianw wants to merge 1 commit into
masterfrom
cli-usability
Open

demianw wants to merge 1 commit into
masterfrom
cli-usability

Conversation

@demianw

@demianw demianw commented Nov 4, 2026

Copy link
Copy Markdown
Owner

What was wrong

Items of #118 addressed here:

  • Existing outputs (tract_math): process_output asks Overwrite file ... (y/N)? with input(); in a batch run (stdin not a terminal) this fails with EOFError. Reproduced: tract_math tract.trk count existing.csv < /dev/null ends in EOFError: EOF when reading a line.
  • Output format (tract_querier): -o out/subj.01 takes .01 as the output format; reproduced: the files are loaded and the queries computed, then the first save fails with OSError: File format not supported.
  • Unclosed files (tract_math): the CSV output is opened without newline='' and never closed; the tractography files opened by argparse.FileType('r') are never closed.
  • Shell: TractQuerierCmd.default echoes every input line (print(line)).
  • Exception messages: TractQuerierSyntaxError.__str__ and TractQuerierLabelNotFound.__str__ return repr(self.value), so str(e) is quoted and a multi-line message shows a literal \n.
  • Help texts: tract_remove_short_tracts says it "subsamples"; tract_affine_transform omits ref_image; tract_point_distance_min_max claims to report distances between consecutive points but prints the shortest and longest tract lengths (tract_length returns the total length of each tract).

The change

  • tract_math: new --force optional flag (handled in the decorator, alongside the existing -- flags) overwrites an existing output without asking. Without --force, if stdin is not a terminal, the command fails with Output file <name> exists, use --force to overwrite it instead of prompting. With a terminal, the prompt is unchanged. --force is documented in the usage text.
  • tract_querier: the output format is worked out right after the options are parsed (the same logic, moved up; the earlier dead assignment of tractography_extension is removed) and must be .trk, .vtk or .vtp (the formats tractography_to_file writes). Otherwise: error: output format '.01' not supported, it must be .trk, .vtk or .vtp; add it to the output prefix, e.g. -o out/subj.01.trk.
  • CSV output is written inside with, opened with newline=''; the FileType handles are closed right after parsing (.name is all that is used afterwards).
  • print(line) removed from shell.default.
  • Both __str__ return str(self.value). Callers in shell.py and cli_tract_querier.py print e.value, so their output does not change; no test matched on the quoted form.
  • Help texts: tract_remove_short_tracts "keeps only the tracts longer than the given length"; tract_affine_transform lists <ref_image> and says it gives the output's affine and dimensions; tract_point_distance_min_max "minimum and maximum tract length".

Behaviour changes: a non-interactive tract_math run with an existing output now exits with an error (exit code 2) instead of an EOFError traceback; tract_querier with an unsupported output extension now errors before loading any file; str() of the two exceptions no longer has quotes.

Not addressed here

  • cli_tract_querier.py still uses optparse with version=0.1 (handled separately).
  • tract_math's parser reads sys.argv while it is built.
  • tract_querier has no overwrite prompt (it overwrites silently), so no --force was added to it.
  • In cli_tract_math.py, TractMathWrongArgumentsError is caught with traceback.print_exc() before parser.error, so the new --force message is preceded by a traceback on stdout.
  • For the maintainer: tract_affine_transform's [invert] is effectively required (the decorator rejects three positional arguments) and bool(invert) is true for any non-empty string, including False; tract_point_distance_min_max's name suggests it was meant to report point-to-point distances. The help now describes what the code does; the code is unchanged.

Testing

  • pytest -q -p no:cacheprovider tract_querier: 45 passed, 1 skipped (master: 42 passed, 1 skipped).
  • New tests:
    • test_tract_math.py::test_tract_math_existing_output: with an existing output and a non-terminal stdin, tract_math ... count out.csv fails, mentions --force and leaves the file untouched; with --force it overwrites it and the CSV reads back as 2 rows with 6783 tracts.
    • test_tract_querier.py::test_tract_querier_unsupported_output_format: -o <tmp>/subj.01 fails with the format error and before Loading files.
    • test_tract_querier.py::test_syntax_error_message_is_not_quoted.
  • Manually: tract_math tract.trk count existing.csv < /dev/null gives the --force message and keeps the file. I did not test the interactive prompt in a terminal (that code path is unchanged).

Merge order

Touches scripts/cli_tract_querier.py in different hunks from #126 (#99); git merge-tree shows no conflict between the two. Small edits (help strings, a usage line, a few lines in main) in tract_querier/tract_math/operations.py, scripts/cli_tract_math.py, tract_querier/shell.py and tract_querier/query_processor.py, which other open PRs on #102, #105, #106, #112, #38, #103, #104 and #115 may also edit: a rebase may be needed depending on which lands first.

Part of #118

🤖 Generated with Claude Code

- tract_math: an existing output file triggered an `input()` prompt, so
  non-interactive runs failed with `EOFError`. Add a `--force` flag that
  overwrites without asking, and fail with a message naming `--force`
  when stdin is not a terminal.
- tract_querier: an output prefix such as `out/subj.01` took `.01` as
  the output format and only failed after all the queries ran. Check the
  format against .trk, .vtk and .vtp right after parsing the options.
- tract_math: close the CSV output (opened with `newline=''`) and the
  tractography files that `argparse.FileType` opens to check they exist.
- shell: remove the debug `print(line)` echoing every input line.
- `TractQuerierSyntaxError` and `TractQuerierLabelNotFound` render their
  message with `str()` instead of `repr()`, so multi-line messages
  print as such.
- Fix the help texts of `tract_remove_short_tracts`,
  `tract_affine_transform` (missing `ref_image`) and
  `tract_point_distance_min_max` (it prints the shortest and longest
  tract lengths).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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.

1 participant