close
Skip to content

Detect swapped arguments, harden error reporting, and add NO_COLOR support#80

Open
mmokrejs wants to merge 2 commits into
lh3:masterfrom
mmokrejs:fix-swapped-args-exit-code
Open

Detect swapped arguments, harden error reporting, and add NO_COLOR support#80
mmokrejs wants to merge 2 commits into
lh3:masterfrom
mmokrejs:fix-swapped-args-exit-code

Conversation

@mmokrejs
Copy link
Copy Markdown

Summary

This PR improves miniprot's error handling for pipeline callers that rely on exit codes and stderr messages.

Commit 1: Detect DNA queries and exit with non-zero code

When the query file contains DNA instead of protein sequences (>80% A/C/G/T/N residues), miniprot now emits a clear error message and returns exit code 1. Previously, swapping the <ref.fa> and <query.faa> arguments would cause miniprot to silently run to completion with exit code 0, producing no useful output.

Commit 2: Harden argument validation, error reporting, and NO_COLOR support

  • Unrecognized options → exit 1: Previously continued silently with exit 0, which is dangerous for pipeline callers where a typo in a flag silently runs with wrong parameters.
  • Missing arguments → specific error: Instead of just dumping the usage text, miniprot now prints [ERROR] missing query protein file or [ERROR] missing arguments before the usage.
  • File errors → include filename: Index load and mapping failure messages now include the filename.
  • Deprecated -s → hard error: Was silently ignored; now exits with a clear deprecation message.
  • Validate all atoi()/atof() calls: Passing invalid values like --outs=abc now produces [ERROR] invalid numeric value 'abc' for --outs instead of silently setting the value to 0.
  • NO_COLOR support (no-color.org): All hardcoded \033[1;31m ANSI escapes across main.c, map.c, bseq.c, and options.c are replaced with MP_COLOR_RED/MP_COLOR_RESET macros that respect the NO_COLOR environment variable and isatty(stderr).

Motivation

When miniprot is called from a bioinformatics pipeline (e.g., via subprocess.run(..., check=True) in Python), the caller relies on the exit code to detect failures. The current behavior of returning exit 0 on swapped arguments, unrecognized options, or invalid numeric values makes it impossible to reliably detect misconfigurations.

Testing

Verified with 9 functional tests covering all error paths and normal operation.

When the query file contains DNA instead of protein sequences (>80%
A/C/G/T/N residues), miniprot now emits a clear error message and
returns exit code 1.  Previously, swapping the <ref.fa> and <query.faa>
arguments would cause miniprot to silently run to completion with exit
code 0, producing no useful output.

The check is performed once on the first batch of query sequences.
If the composition is >80% nucleotide characters, the mapping is
aborted immediately with:

  [ERROR] query file appears to contain DNA, not protein sequences.
          Did you swap the arguments? Usage: miniprot <ref.fa> <query.faa>

This makes it straightforward for wrapper scripts to detect the
failure via subprocess return code.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Apr 26, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 27 complexity · 0 duplication

Metric Results
Complexity 27
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@mmokrejs mmokrejs force-pushed the fix-swapped-args-exit-code branch 6 times, most recently from 9893016 to c9b3e4b Compare April 26, 2026 07:58
Build on the DNA-in-protein detection from the previous commit:

- Unrecognized options now exit non-zero (previously continued silently
  with exit 0, dangerous for pipeline callers)
- Missing query/reference file arguments produce specific error messages
  instead of just dumping usage
- Index load and mapping failures include the filename in the error
- Deprecated -s option is a hard error instead of a silent no-op
- ALL atoi()/atof() calls are now validated: passing invalid values like
  --outs=abc is caught with a clear error message and exit code 1
- Respect NO_COLOR (https://no-color.org/): ANSI color escapes are
  suppressed when NO_COLOR is set or stderr is not a terminal.  All
  hardcoded \033[1;31m escapes across main.c, map.c, bseq.c, and
  options.c are replaced with MP_COLOR_RED/MP_COLOR_RESET macros.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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