Repository navigation
Skip ';' comment lines before the first FASTA record - #122
adamjohnwright wants to merge 1 commit into
Conversation
parse_fastx_reader rejected a FASTA file whose first lines start with ';' (the original FASTA comment convention): "Expected '@' or '>' at the start of the file but found ';'". This breaks e.g. salmon index on such references. Skip those lines before the first '>' record; they are still an error before a FASTQ record. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Hi @adamjohnwright. Supporting comments at the beginning of the file (and possibly followed by blank lines) seems like a low-risk addition to needletail but I'm not sure if that is the complete spec for Pearson-style FASTA files as there seem to be examples of other parsers and files that have comments in-between records, empty lines between comments/records, and whatever this is on Wikipedia (the LCBO example). I'd like to avoid creating yet another subtle variation of FASTA parsing by only implementing part of the Pearson style comment "spec". I also can't find any examples in that repo with comments (the LCBO example in that repo seems to have had its comments stripped https://github.com/wrpearson/fasta2/blob/master/lcbo.aa) 😭 . If the goal is to maintain backwards compatibility with Salmon pre-Rust rewrite, then I think needletail would need to skip everything before the first record: https://github.com/COMBINE-lab/salmon/blob/a2f6912b3f9f9af91e3a4b0d74adcb3bdc4c9a32/include/kseq++.hpp#L572 and inter-record comments would get included as part of the sequence. I believe htslib behaves the same way. This might be okay with the current implementation (it preserves the most backwards-compatibility, and skipping lines at the beginning of the file won't significantly impact performance). So I'm not really quite sure what the best option is here. FASTA format is a can of worms and the probably easiest way to maintain a performant parser is to limit support to a strict subset. I'm open to ideas though. |
|
Thanks @audy, that's a really helpful analysis. Skipping everything before the first record would work well for us, and it avoids defining a partial Pearson variant: it's the existing kseq behaviour (old salmon, and in our testing BWA, minimap2, seqtk, kallisto, |
|
To be clear, the existing kseq.h behaviour is to start parsing FASTA/Q records from the first |
|
Thanks @jmarshall, that's an important detail. To be precise about what we'd propose for needletail: skip whole lines until the first line that starts with |
parse_fastx_readerrejects a FASTA file whose first lines start with;(the original Pearson FASTA comment convention):This currently breaks tools built on needletail. For example, salmon 2.8.0
salmon index -t ref.fafails with exactly this error, via piscem-rs.This PR:
get_fastx_readerhandles a leading;by skipping;lines, then returning aFastaReaderfor the rest.UnknownFormat, since FASTQ has no comment lines.EmptyFile.>or@take exactly the same path as before.One side effect to be aware of: for files with a comment block, the reader's own byte and line positions count from the first
>rather than from the start of the file.Tests: four new tests in
parser::test:cargo test --all-featurespasses (Rust 1.99), andcargo fmtandcargo clippy --all-featuresare clean.Context: we maintain the FAIR Header Reference genome (FHR) metadata standard (https://doi.org/10.1093/bib/bbae122). It stores provenance as
;~lines before the first record. The same narrow rule is proposed for htslib, htsjdk, seqkit and others.This change was prepared with an AI coding assistant (Claude Code) and reviewed by me.