Add custom/resolvetaxonomy module - #12817
Merged
erikrikarddaniel merged 4 commits intoAug 26, 2026
Merged
Conversation
Resolves a set of sequences' taxonomy from either an explicit tab-separated file or, if omitted, embedded GTDB-style taxonomy text in each FASTA header (>id taxonomy;string). Headers are always stripped down to a bare id in the output sequences. Generalises nf-core/sativa's existing local resolvetaxonomy module into a shared component, adding a taxonomy_required boolean: sativa needs every record to end up with a resolved taxonomy (fails the run otherwise, same as before); nf-core/phyloplace's --taxonomy is fully optional, so it needs records with neither source to be dropped with a warning instead. Generated by Claude
CI's conda profile failed to solve bioconda::biopython=1.84 -- bioconda's own biopython recipe is frozen at 1.70; every release from 1.71 onward publishes under conda-forge instead. Confirmed via the Anaconda API's package-files endpoint for both channels (the versions summary endpoint used by nf-core lint's own bioconda_version check returned stale/truncated data for the bioconda package, which is what let this slip through initially). Generated by Claude
conda | 7's CI shard failed on a snapshot mismatch: python's own patch version (3.12.12 in the pre-built container vs 3.11.16 from a fresh conda solve) legitimately differs between profiles, since environment.yml only loosely pins python (=3.11) while the container was built independently. biopython is pinned exactly (=1.84) in both, so reporting that instead removes the drift at the root rather than working around it in the test. Generated by Claude
erikrikarddaniel
marked this pull request as ready for review
August 26, 2026 11:51
leapicard
self-requested a review
August 26, 2026 14:10
Contributor
|
As far as I see, everything looks good, but it's my first time reviewing so maybe someone else should take a look? |
leapicard
approved these changes
Aug 26, 2026
| def taxonomy_in = taxonomy ? "${taxonomy}" : '' | ||
| """ | ||
| python3 - "${taxonomy_in}" "${sequences}" "${taxonomy_required}" "${prefix}.resolved.tax" "${prefix}.resolved.${sequences.extension}" "${prefix}.warnings.txt" << 'PYEOF' | ||
| import sys |
Contributor
There was a problem hiding this comment.
If I have one suggestion, it would be to actually put the python script in a resources/usr/bin folder to make main.nf cleaner
Member
Author
There was a problem hiding this comment.
It is quite long, which would argue for that. Personally, I find it good to have the code here as it then ends up directly in .command.sh which I think makes it easier to debug by stepping through. But quite a personal opinion.
Thanks for the review!
This was referenced Aug 26, 2026
erikrikarddaniel
added a commit
to nf-core/phyloplace
that referenced
this pull request
Aug 27, 2026
…sent Closes #66. When refseqfile is FASTA and no --taxonomy file is given, taxonomy is now derived from each reference sequence's own header, GTDB single-file style (>id taxonomy;string), via the new shared custom/resolvetaxonomy module (nf-core/modules#12817), instead of proceeding with no taxonomic classification at all. - workflows/phyloplace.nf: sniff whether refseqfile is FASTA before invoking the module at all (other HMMER-supported formats have no room for embedded taxonomy text); run it per row with taxonomy_required=false, so a row with neither an explicit file nor embedded text still proceeds without classification rather than failing; reset an empty resolved-taxonomy file back to `[]` so GAPPA_ASSIGN's existing ext.when skip keeps working. - conf/modules.config: publish resolvetaxonomy/ output. - nextflow_schema.json, docs/usage.md, docs/output.md: document the new behaviour. - New conf/test_embedded_taxonomy.config + tests/embedded_taxonomy.nf.test, using a new nf-core/test-datasets fixture (erikrikarddaniel/test-datasets embedded-taxonomy-fixture branch, PR pending) -- an embedded-taxonomy variant of the existing PF14720_seed.alnfaa reference alignment, same 138 sequences and tree, so no new reference tree was needed. Verified locally against the fork branch before the fixture PR merged; produces the same taxonomic classification (6/5/2 row counts) as the existing explicit --taxonomy test profile. - Existing pipeline-level snapshots (default, clustalo, hmmfile, mafft, phyloplace_input, phylosearch_input) updated: every prior md5 is unchanged, only the new resolvetaxonomy/ files were added, since refseqfile/taxonomy were never themselves directly published under their own names before. The vendored modules/nf-core/custom/resolvetaxonomy copy also carries a fix for a real bug found while testing this (an extensionless refseqfile URL in the phylosearch test fixture produced a malformed output filename) -- applied here ahead of the corresponding upstream PR (nf-core/modules, resolvetaxonomy-extensionless-input branch, not yet merged) landing; `nf-core modules update custom/resolvetaxonomy` once it does should be a no-op confirming the two match. Generated by Claude
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
2 participants
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.
PR checklist
topic: versions- See version_topicslabelnf-core modules test custom/resolvetaxonomy --profile dockernf-core modules test custom/resolvetaxonomy --profile singularitynf-core modules test custom/resolvetaxonomy --profile condaDescription
New module resolving a set of sequences' taxonomy from either an explicit tab-separated file, or, when omitted, embedded GTDB-style taxonomy text in each FASTA header (
>id taxonomy;string), matching GTDB's own single-file reference convention. Headers are always stripped down to a bare id in the output sequences, since some downstream tools keep the whole header line as the leaf/sequence name.This generalises nf-core/sativa's existing local
resolvetaxonomymodule into a shared component. Two pipelines will consume it:--taxonomy/--refseqfilepair, but its--taxonomyis fully optional -- no taxonomy at all is a legitimate, common case, so it must not fail the run.To serve both, the module gains a
taxonomy_requiredboolean input that wasn't in sativa's local version:truereproduces sativa's existing behaviour unchanged -- a record with neither an explicit-file entry nor embedded header text fails the run.false(phyloplace's case) drops such records from the resolved taxonomy output instead, with a warning, rather than failing.Sequence format (FASTA, Clustal or PHYLIP) is sniffed from content, same as sativa's original module. Detecting whether a given input format has "room" for embedded taxonomy at all (FASTA does, PHYLIP/Clustal effectively don't) and gating whether to invoke this module is left to the calling pipeline, not handled inside the module.
Test data
Fixtures added in nf-core/test-datasets (
modulesbranch): erikrikarddaniel/test-datasets#TBD (branchadd-resolvetaxonomy-fixtures, PR pending) -- five small hand-authored files underdata/generic/fasta/anddata/generic/tax/covering all three taxonomy sources plus thetaxonomy_requiredtrue/false split. This PR's own test file references them viaparams.modules_testdata_base_path; tests were run locally against the fork branch before those fixtures were merged and passed twice independently (not just once with--update-snapshot).Notes
nf-core modules lint custom/resolvetaxonomyis clean apart from onebioconda_versionfalse positive onbiopython=1.84-- the Anaconda API query the linter uses currently returns a stale/truncated version list for thebioconda::biopythonpackage (tops out at1.70), unrelated to this module; the same pin is already in production use in nf-core/sativa's local module.Drafted with help from Claude Code.