v2.3.0 PR - #116
Open
BioWilko wants to merge 18 commits into
Open
Conversation
…y slow to account for an extremely rare failcase
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.
Reduces runtime and intermediate storage for large inputs, and fixes several taxonomy-parsing bugs surfaced by recent taxonomy versions.
Single-pass read extraction
Previously each output group meant its own process and its own full decompress and parse of the FASTQ. Now:
extract_taxa: one process per sample covering all kreport splits, driven by a--report_configJSON (was one process per split).extract_fractions: 6 processes collapsed to 1.virus_and_unclassified,virusandhuman_filteredare extracted in one pass via--fraction_config.KrakenAssignments.get_read_maps()builds N independent read maps in a single pass over the assignment file.extract_utils: newTaxonWriter(LRU-capped filehandles plus buffered writes) bounds memory and fd use; per-taxon running sums replace per-read quality/length lists; fastermean_phred.HCID
check_hcid.pynow takes read sequences from the SAM rather than re-decompressing the FASTQ, reverse-complementing flag-16 records to restore original orientation. The FASTQ is no longer staged into the process at all.rammap_hcid), with a newdockerfiles/rammap/Dockerfilepulling the pinned v1.1.2 release binary. Verified byte-identical SAM output vs minimap2 2.26.Compression and I/O
bgzip/pigzreplaced withcrabzthroughout (faster, lower CPU, better ratio).fastpnow streams via FIFO/stdout straight into crabz instead of writing an uncompressed temp FASTQ.get_total_length.pyand its two processes, an entire extra FASTQ pass. Total length now comes from the fastcat per-read stats and from the extraction pass itself.check_reads.py: bounded-window duplicate/interleave detection with early exit, plus a long-read fast path.concatenate_reads.py: batched writes; fixedgzip.open(..., mode="w")to"wt".Taxonomy and report correctness
report.py: domain detection by name (modern taxonomy usesR1/R2rather thanD); newsimple_rankso numbered ranks (G1,D2, etc.) match correctly, previously these were missed in per-taxa read files.split_kraken_report.py: ancestor backfill ordering fix.taxonomy.py:childrenas list rather than set;load_entries_from_nodesnow honours itstaxon_idsfilter instead of loading all ~2.6M nodes."Select domain" dropdown showed random mix of taxa for modern taxonomy
NCBI retired
superkingdomin 2024: Bacteria/Archaea/Eukaryota are now rankdomain, Viruses isacellular root, withrealmclades beneath it.aggregate_lineages_bracken.py's hardcodedRANKSallowlist silently dropped any node whose rank wasn't listed, so the real domain nodes vanished and the next allowed rank —kingdom— became the top level of the lineage tree. The dropdown is populated from that tree's top-level keys, so it offered kingdoms and clades: on the test fixture,Pseudomonadati,Opisthokonta(a clade),Heunggongvirae. NowBacteria,Eukaryota,Viruses,Unclassified.RANKSgainsacellular root,domainandrealm, and keepssuperkingdomso pre-2024 dumps still resolve.cellular rootis deliberately excluded — including it would collapse all three cellular domains under a single meaningless node.domain, so it previously contributed to no top-level node at all. Sankey percentages will shift slightly, and the top-level keys in*.kraken.jsonare renamed.bin/ranks.pyholds the vocabulary once (RANKS,DOMAIN_RANKS,UNCLASSIFIED_RANK,KNOWN_DOMAIN_NAMES), imported byaggregate_lineages_bracken.pyandreport.pyand injected into the report's JS bymake_report.py. Three copies previously had to agree by hand, and the JS copy drives both the#rank-selectoptions and the sankey depth filter — where an unrecognised rank yieldsindexOf() == -1and passes every filter, so drift showed up as nodes ignoring the rank selector.sankey.js: nodedomain_namewas derived fromval.rank == "superkingdom", which after the rename matched only the syntheticUnclassifiednode — leaving every tooltip'sDomain:fieldnull. Now tested againstdomainRanks.aggregate_lineages_bracken.py: theUnclassifiednode's rank wasRANKS[0], incidentallysuperkingdom; now an explicitUNCLASSIFIED_RANK.aggregate_lineages_bracken.py:previous_rankwas only assigned on the "node already exists" branch, so thespecies→subspeciesfallback for off-allowlist ranks only fired when the parent species node happened to already be in the tree.report.py:get_domains()re-derived domains fromsimple_rank == "D", which the module's own comment explains is unreliable on modern dumps; it now returns theself.domainsmap already built at load time. Also dropped the strayentry.print()side effect from it and fromget_tips()/get_rank_entries().Containers and config
fastcat/fastp/jqplus crabz;get_versionsreadsconda-metavia jq (Wave images ship no package manager).publish_dir_modeparam (defaultcopy) replacing hardcodedmode: 'copy'.docker-build-push.yml; version bumped from v2.2.1 to v2.3.0.Testing