Repository navigation
Mothur 1.48.6 upgrade - #8904
Open
bernt-matthias wants to merge 56 commits into
Open
bernt-matthias wants to merge 56 commits into
bernt-matthias wants to merge 56 commits into
Conversation
The old version_command parsed 'mothur "#help()"' output which no longer works in mothur 1.48; use 'mothur --version' instead. Assert on the logged version with a regex since the runtime binary reports 1.48.x (the conda 1.48.6 package binary self-reports 1.48.5).
make.design and parse.list no longer exist in mothur 1.48.
Replace the removed topdown parameter with method (simple/unoise/tree/ deblur) and clump; add the method-specific parameters alpha, delta, error_rate, indel_prob, max_indels and error_dist.
mothur 1.46+ redesigned cluster.split: the splitmethod/column/phylip/large options (splitting an existing distance matrix) are gone and the command now requires a fasta file, a taxonomy file and a name file or count table. It splits the sequences by taxonomy first and clusters within each group. - expose the 1.48 parameters (taxlevel, method incl. weighted/agc/dgc, classic, cutoff, precision, dist, cluster, runsensspec) - add the vsearch requirement for the agc/dgc methods - declare method-specific outputs (fn/nn/an/wn, opti_<metric>, agc, dgc) and the split-mode outputs (per-group dist/name collections, singletons, .file) - document the upstream crash when no taxonomy group has any distance below the cutoff, and why resuming from the .file is not possible in Galaxy - test with a 32-sequence HMP mock alignment that forms real taxonomic clusters, covering furthest, opticlust, agc, cluster=false and count-table input
- make.contigs: drop the rename parameter (removed in 1.48; name normalisation is now a session-wide set.dir(modifynames=) setting), pass qfile=T since 1.48 made qual output opt-in, follow mothur's renamed *.contigs_report output and replace the dropped *.groups output with the new *.contigs.count_table. Regenerate the fixtures, which gained an ee= field on the fasta headers and an Expected_Errors column in the report. - get.oturep: drop the label and large parameters; 1.48 replaced label with cutoff (which also accepts a range such as 0.01-0.03). - chimera.slayer: drop search and processors. Slayer now searches an internal kmer DB, so the BLAST helper symlinks are gone too; expose ksize/realign/removechimeras instead and follow slayer.fasta output. - sort.seqs: drop count and group, which 1.48 no longer sorts. - sens.spec: names= was never a parameter, the argument is name=; also drop the duplicated label block. - trim.flows: order= was renamed to floworder= in 1.48. - drop processors from chimera.bellerophon, chimera.ccode, chimera.check, chimera.pintail, get.communitytype, seq.error and shhh.flows, none of which accept it any more. - seq.error: remove the dead output_sel references left behind when the output picker was deleted in 2016.
accnos is parsed by split.abund but never used, and it is not in the command's parameter list, so selecting it only earned a 'not a valid parameter, ignoring' warning while the .accnos outputs were written unconditionally anyway. Emit those outputs unconditionally instead. pre.cluster: add the missing when branch for the default simple method and pin it in the tests, and drop an invalid width attribute.
The 1.48 make.contigs output gained an ee= field on the fasta headers and an Expected_Errors column in the report, so byte-comparing against the shared Mock_S280 fixtures (which the rest of the suite still uses as inputs) no longer works. Assert the 1.48 content directly instead, and restore the shared fixtures to their previous content. Note for a follow-up: those fixtures are 1.39.5-era and their qual file is not readable by modern mothur at all. QualityScores(ifstream&) calls gobble() after the sequence name, which eats both the trailing tab and the newline of a bare '>name<TAB>' header, so the scores line is taken as the comment string and every record desynchronises into 'expected a number and got ><next name>'. Regenerating the qual fixtures also shifts the md5s asserted by get.seqs, remove.seqs, screen.seqs, make.fastq, summary.qual, seq.error, sort.seqs, get.lineage and remove.lineage, so that needs its own pass.
…ixtures
Drop tools/mothur/.lint_skip: every one of its nine checks is now clean, so
planemo lint reports no findings for the suite at all.
Lint-driven changes, applied suite-wide (126 wrappers):
* TestsExpectNumOutputs (was 301 warnings): added expect_num_outputs to all 316
tests. The values are derived from the tool's own output filters, evaluated
the way Galaxy does (galaxy/tools/execution_helpers.py:filter_output): a
filter withholds its dataset only when it evaluates to False without raising,
booleans arrive as the strings 'true'/'false', and a whole collection counts
as a single output.
* TestsParamInInputs / TestsCaseValidation (was 174): nested the test <param>s
back into the <conditional>/<section>/<repeat> that declares them. Done as a
textual rewrite so the hand formatting and the explanatory comments survive;
verified by diffing the parsed (name, value) sets against HEAD for every file
(only deliberate fixes differ).
* OutputsFilterExpression (8): repaired unbalanced parentheses and the stray
leading spaces in split.abund, get.otus, get.communitytype, unique.seqs.
* OutputsFormatSourceReference (12): filtered on the parameter that really is
declared (input|dist, infile|otu, merge|inputs, ...).
* OutputsNameInvalidCheetah (11) and OutputsLabelDuplicatedFilter (11): renamed
the dotted output identifiers and de-duplicated the labels.
* InputsOptionsFiltersAllowedAttributes (8): dropped name= from the
unique_value/sort_by filters.
* HelpInvalidRST (1), and dead filters that never could have worked:
chimera.check filters on svg_cond['svg'], get.sharedseqs on infile.get('fasta')
and indicator.seqs on withdesign['tree'] instead of names Galaxy never defines.
Fixtures and commands that mothur 1.48 itself made unusable:
* ContigsReport::read needs the Expected_Errors column; a 7-column report makes
screen.seqs spin forever writing contigsreport.good.[extension]. Ship a 1.48
make.contigs report instead and update test-contigsreport-format to 8 columns.
* screen.seqs' alignreport/contigsreport output patterns lost their separating
comma upstream (screenseqscommand.cpp:148-149), so mothur writes the literal
name 'good.[extension]'. Rename the file after the run.
* mothur 1.48 refuses design files without a header row; add the 'group' header
to amazon.design and toymothur.design2, which unblocks amova, anosim, homova
and indicator (they silently produced nothing before).
Still failing end-to-end (need their own pass, all pre-existing): 59 tests whose
fixtures/output names assume older mothur behaviour, e.g. align.seqs' .align.report
which 1.48 only writes on request, libshuff/seq.error/unifrac outputs, make.fastq,
and the 1.39.5-era qual fixtures feeding sort.seqs.
Found by running every wrapper's tests against mothur 1.48.6 with a harness
that mirrors Galaxy's command rendering (staged inputs, cwd = job dir,
configfiles, macro expansion, output names in the Cheetah namespace).
* filter.seqs: 1.48 reads multiple `fasta` entries separated by '|'
(filterseqscommand.cpp uses util.splitAtChar(fasta, ..., '|'); 1.39.5 used
splitAtDash), so the repeat's files were passed as one bogus filename
("Unable to open fasta.dat-fasta0.dat").
* make.group: 1.48 added `format` and defaults it to `count`, which writes
merge.count_table instead of the *.groups file the wrapper declares and
downstream tools consume. Pass format=group to keep the previous output.
* get.lineage / remove.lineage: the command interpolated `$taxon.value`, but
`taxon` is a plain text param -- Galaxy hands the command its value, not a
parameter object, so rendering died with "cannot find 'value'". Use $taxon.
* get.lineage / remove.lineage: test 2 fed the 7-column
Mock_S280...contigs.report as a mothur.align.report; 1.48's AlignReport
reader cannot parse it and loops forever. Use the 16-column
Mock_S280...trim.contigs.good.align.report instead, and assert the pick
output with <assert_contents> (line count + header) rather than an md5 that
was never derived from a valid input.
Verified: filter.seqs t1-t2, make.group t1-t3, get.lineage t1-t5 and
remove.lineage t1-t5 all run to rc=0 under 1.48.6; planemo lint reports no
findings on the four files.
Replace the suite's single @WRAPPER_VERSION@ token with the IUC convention:
macros.xml
@TOOL_VERSION@ 1.48.6 (also used for the conda requirement)
@VERSION_SUFFIX@ 0
@Profile@ 25.1
every tool
<tool profile="@Profile@" ... version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@">
All 127 tools (126 mothur wrappers plus taxonomy-to-krona) now carry
profile 25.1 instead of 16.07, so they opt in to the current Galaxy
behaviours (strict Cheetah/interpreter, exit-code and stdio handling,
sanitized identifiers) rather than the 16.07 compatibility path.
Per-tool historical suffixes (.1/.2 on chimera.vsearch, make.contigs,
screen.seqs and summary.single) collapse into the suite-wide suffix, which
this release bumps as a whole.
planemo lint --fail_level warn: 127 tools linted, 0 findings.
…tory
mothur caches the k-mer index next to the file it indexes:
datastructures/kmerdb.cpp:35
kmerDBName = fastaFileName.substr(0, fastaFileName.find_last_of(".") + 1)
+ char('0' + kmerSize) + "mer";
The wrapper passed reference/name/group/count straight through as absolute paths, so
running it left left.<ref>.7mer / right.<ref>.7mer in the dataset directory (in Galaxy:
the object store, which may be read-only and is never the job dir). Symlink all four
inputs into the job dir and pass the local names, matching the rest of the suite.
Found by the test sweep, which started writing those files into tools/mothur/test-data
once the harness handed the tools absolute paths like Galaxy does. chimera.slayer t1
now produces its chimeras + accnos instead of just the log.
Both were found by the end-to-end test sweep once the harness stopped copying
inputs into the job dir and handed the tools absolute paths like Galaxy does.
test-data/amazon.groups had no LF byte at all -- it used 97 bare CR line
separators, i.e. classic-Mac line endings (probably mangled by a tool that
converted the file). 1.48 replaced mothur's ad-hoc readGroupFile() with the
GroupMap reader and is stricter about it, so make.shared either cross-checked
garbage ids ("U68667 is in your listfile and not in your groupfile") or saw no
group at all ("requesting groups not present in files"). With LF endings both
tests run clean and the outputs match every md5 the tests already declared
(get.groups, name/count/list outputs included), which confirms the declared
expectations were derived from the LF version all along.
test-data/HMPv5-v3_1368-1369.oligos has only a forward primer, and 1.48's
fastq.info runs split.groups after parsing whenever an oligos file is given
(fastaqinfocommand.cpp:126 sets split=2; :335 then builds a count file only if
seqGroups is non-empty, :1096 warns but does not skip). With no group names in
the file no count table is written and split.groups fails with four
"[ERROR]: Could not open" lines, which the wrapper's stdio block rightly treats
as fatal. Give the primer an explicit group (the file already documents that
'#' lines are comments), so the fixture keeps testing primer trimming.
- align.seqs: 1.48 writes fasta.align_report (underscore); drop blast from
search/align (removed in 1.48, aligncommand.cpp:182/185); suffix/gotoh test;
flip_accnos is empty (1.48 deletes blank accnos); report test md5 -> assert_contents
- tree.shared: single label + <=2 calcs (upstream bad_alloc on multi-label shared=,
phantom {parameter}.NN.tre files); limitation documented in help; assert by
calc name + label regex
- unifrac.weighted/unweighted: trailing space in from_work_dir='tree.dat*.dist '
matched nothing; 1.48 flips the weighted UWScore sign -> shape assertions;
unweighted tests always supply group= (1.46/1.48 segfault in file-input mode)
- pairwise.seqs/dist.seqs/phylo.diversity: md5 -> assert_contents (line counts,
shape regexes, pair-name checks)
- summary.shared: fix swapped #if $label: / #if $groups: branches; subsample= runs
write only the ave/std dist files -> <filter> on summary + t2 asserts the 22 dist
files; sharedobserved dropped from t3 calc list (not a valid estimator)
- get.coremicrobiome: t2 expects 4 elements (Utils::getNextRelabund also processes
the label after the requested ones, 0.55)
- summary.qual: t1 uses Fasting_Example1.qual (Mock_S280...trim.contigs.qual has a
trailing tab after each name that 1.48 mis-reads)
- document the Utils::getNextShared label-restriction regression (new in 1.46+)
in a test comment; assertions check requested labels are present
All 41 Batch-1 tests pass in the Galaxy-faithful harness (pipe mode,
GALAXY_SLOTS=1); planemo lint --fail_level warn clean (0 warnings/0 errors).
Suite-wide: - Append `s/,)$/)/` to the existing `s/ //g` sed in every tool command. A trailing comma before the closing paren (e.g. `foo=bar,)`) makes mothur >1.42 segfault: optionparser splitAtComma yields an empty token and the following value[0]/value[len-1] index dereferences it. 126 wrappers. Chimera batch (b2: 17/17 tests pass): - chimera.ccode: mask param symlinked the wrong member ($mask_cond.input -> $mask_cond.mask). - chimera.pintail: prepend set.dir(mothurfiles=.) so getDefaultPath() is non-empty (chimerapintailcommand dereferences getDefaultPath()[0] when the optional conservation file is absent and MOTHUR_FILES is undefined in the conda build); separate the two echos with && so the Galaxy line-collapse does not merge them; convert tests md5->assert_contents. - chimera.vsearch: out_count from_work_dir template.*vsearch.pick.count_table -> template.*vsearch.count_table (1.48 emits the former without "pick"); convert tests file=/md5 -> assert_contents; reference-mode (query==reference) now flags 0 chimeras in 1.48, so that test asserts an empty accnos. Post-harness-fidelity Batch 1 regressions (has_line_matching is whole-line): - summary.qual, summary.shared: fix row regexes to match full lines (empty group column -> double tab; allow negative calculation values). Harness-only (not committed): mtest.py Unset.__eq__ mirrors Galaxy's None binding for unset data params; _count_ok presence semantics; compare_files sort fix.
All 22 tests in the taxonomy family pass against a local mothur 1.48.6.
classify.seqs:
- tree.sum is a blank-line-separated record file whose first line is the
runtime version string; the test asserted `has_text "@TOOL_VERSION@"`
(i.e. 1.48.6) but the conda binary reports its own build number there, so
that assertion could never hold. Replaced the md5 + version text with a
version-agnostic `^#\d+\.\d+\.\d+$` header assertion.
classify.otu:
- mothur 1.48 honours label= for the consensus taxonomy but still writes a
tax.summary for every label in the list file, so the three label-restricted
tests produce 20 collection elements, not 3. Documented; counts fixed.
make.biom:
- 1.48 writes HDF5 biom by default whenever the HDF5 API is compiled in
(makebiomcommand.cpp:255), which silently broke the wrapper: the collection
was declared format="biom1" (plain-text/JSON). The `output` parameter was
never exposed. Added it (simple/hdf5, default simple so existing users keep
the biom1 datatype), split the collection in two with <filter>s on it, and
added a test for the hdf5 branch (verified the file really is HDF5).
- Label subset test produced 5 files rather than 3 (same upstream
over-production as classify.otu) -- documented, count fixed.
- The constaxonomy test paired final.tx.1.cons.taxonomy (62 OTUs named 1..62)
with final.tx.1.subsample.1.pick.shared (44 OTUs named Otu01..Otu44); 1.48
aborts on the mismatch ("your constaxonomy file contains 62 otus and your
shared file contain 44 otus"). Added a matching fixture
final.tx.1.subsample.1.pick.cons.taxonomy derived from that shared file.
mimarks.attributes:
- Upstream regression in 1.48.6: the port to the Command() base class dropped
`outputTypes["source"] = tempOutNames;` from the constructor
(source/commands/mimarksattributescommand.cpp; 1.46 and earlier have it and
every other 1.48 command still initialises its output types). So
getOutputFileName("source", ...) logs "[ERROR]: this command doesn't create
a source output file." and returns "", mothur exits 0 and no *.source file
is written, leaving an empty Galaxy output. The md5 assertion is replaced
by a logfile assertion that documents the breakage (and will fail once
upstream fixes it); the limitation is called out in the help text.
cluster
- opti/vsearch only write a .list (clustercommand.cpp runOptiCluster); rabund
and sabund come from the linkage algorithms only, and are suppressed by a
count file as well. Filter both outputs on
method in (furthest, nearest, average, weighted) and not count; drop them
from the two opti tests and re-purpose the stale duplicate test as a real
average-linkage test on the phylip matrix (which does produce 0.03 labels).
make.group
- 1.48 collects the assignments in a std::map, so the output is sorted by
sequence name and a sequence appearing in several input files is reported
once (last group wins). Tests now use two disjoint fastas (the previous
fixture-driven test silently produced a 10-line file), assert inline, and
the two orphaned expected-output fixtures are dropped.
label= cut points
- mothur treats label= as a set of cut points and keeps processing every
label above the last one (label=0.22 on amazon.an.shared -> 0.22..0.55).
Same in 1.46, i.e. not a 1.48 regression, but it invalidated a pile of
element counts and expected outputs. Use the topmost label where a single
label is wanted and document the behaviour in dist.shared/summary.single.
summary.single
- with subsample=T mothur only writes the ave-std summary, so the plain
summary output is now filtered on `groupmode and subsample['use'] != 'yes'`
(a bare `not subsample['use']` is always false, 'no' being a truthy
string in Galaxy's filter language).
md5 -> assert_contents
- count.groups (grp_count is a logfile tail), make.shared, dist.shared,
get.sharedseqs, heatmap.bin, normalize.shared, cluster.classic,
get.rabund, get.sabund, get.relabund, collect.single, collect.shared,
summary.single.
cluster.classic
- 1.48 prints labels without the trailing zero (2.1 where 1.46 wrote 2.10).
planemo lint --fail_level warn tools/mothur: 1397 checks, 0 warnings/errors.
Fix the sequence/OTU manipulation tools for mothur 1.48 and convert their md5 output assertions to inline assert_contents. Behaviour changes handled: - pcr.seqs: start/end are 1-based; flanking outputs are only written when sequences are actually discarded; alignment output may contain N. - split.abund: 1.48 no longer writes per-group files, so the dead groups parameter is dropped and the tool points at split.groups instead. - trim.seqs: name files are converted to count tables, so *.trim.count_table replaces *.trim.names; allfiles requires a count table. - split.groups: 1.48 names per-group files after the input extension. - get.oturep: only the first/unique label is emitted regardless of cutoff and group/count are mutually exclusive; count_out/names_out filters now follow the method switch (matrix_condition vs. top level). - get.lineage/remove.lineage: 1.48 output patterns leave a literal "[extension]" in the align report name and hard-code some extensions; the wrappers rename the affected files. - get.label/get.group/count.groups: fixed-offset logfile parsing broke because the 1.48 logfile header is shorter; use awk extraction instead. - get.groups/remove.groups/screen.seqs/deunique.seqs: dead <filter> expressions (attribute access on conditionals) fixed. - merge.groups: the design file now needs a "group" header row. - trim.flows: per-sample flows are named flow.trim.<sample>.flow; the collection pattern is anchored and the md5s became assertions. - sub.sample: name input yields a count table (renamed to one stable name); label selection cannot stop mothur from extending the label series. - pre.cluster: name input yields a count table; the map file columns changed. - sort.seqs: the .qual test fixture was rewritten (>name<TAB>expected_errors).
Replace 131 `md5=` test expectations with inline `assert_contents` (`has_n_lines` plus sampled exact lines) in 45 wrappers whose tests already pass with mothur 1.48.6, so the expectations are readable and reviewable instead of opaque checksums. No test semantics were changed: every converted test was re-run and still passes.
…sertions
Finish the md5 -> inline assert_contents conversion: the suite no longer
contains a single md5= assertion (was 352 across 80 wrappers). The last ten
red tests needed more than a conversion, so they are triaged here as well.
Stale fixtures / expectations, behaviour identical in mothur 1.46 and 1.48:
* degap.seqs emits the sequences in a thread-dependent order, so compare its
fasta order-insensitively (sort="true").
* chimera.slayer with a names file aborts: the count table mothur derives from
the names file loses the last sequence (E.faecalis.2), which trips the
abundance sort ("is in your fastafile, but is not in your countfile"). Use
the equivalent count table instead, compare sorted, and add the missing row
to the fixture.
* bin.seqs with a name and a group file reduces the list to its unique label
before binning, so that combination can only produce a single unique.unique
bin. Assert what mothur actually writes.
* filter.shared and metastats: label is a set of cutoffs, not a filter, so
mothur keeps every label of the shared file above the lowest one requested
(8 instead of 3, 15 instead of 3). metastats also names its files
<label>.<set2>_<set1>, i.e. 0.23.dalek_tardis.
* pcoa: the coordinates changed in the 4th decimal since the 1.39-era fixture
(eigensolver), so assert the table shape and a few rows.
* The test-shared-format macro now also accepts ASV shared files, whose header
column is numASVs instead of numOtus.
Two genuine mothur 1.48 command bugs, both documented in the wrappers:
* chimera.uchime in reference mode never creates the formatted query fasta it
hands to uchime (chimerauchimecommand.cpp driver() reads "[fasta]temp",
which only the de novo branch writes), so uchime receives an empty query and
silently reports zero chimeras. The wrapper pre-creates that file; the output
is then identical to mothur 1.46.
* merge.sfffiles chops the last character off the name it is given (the sff
output pattern is "[filename],", merge_sfffilescommand.cpp), so "merged.sff"
ends up as "merged.sf". The wrapper writes a scratch name and renames the
result to the dataset path Galaxy handed it.
seq.error's second test needed the reordered .qual fixture from batch 5;
mothur 1.48 segfaults when the qual and fasta record orders differ.
Verified with the Galaxy-faithful renderer: all 317 tests of the suite pass,
planemo lint --fail_level warn is clean (EXIT 0).
Adds wrappers for make.clr, kruskal.wallis, merge.otus, sparcc and estimator.single, the 1.48 commands that were usable outside of an interactive session and could be exercised locally: - make.clr: centred log-ratio transform of a shared file. - kruskal.wallis: per OTU Kruskal-Wallis test over the design file, returned as a collection keyed by label. - merge.otus: merges OTUs sharing a taxonomy, takes shared/list/relabund and returns the merged table with format_source, plus the merged consensus taxonomy per label. - sparcc: compositional correlation network, three tables per label. - estimator.single: richness estimators; the output file name is the name of the calculator, so the collection pattern lists them. New test fixture amazon.treatment.design (group/treatment with the group header mothur >= 1.43 requires). Drops the stale make.design hints from get.groups, lefse, metastats and rarefaction.shared: make.design no longer exists in mothur 1.48 and the help text now describes the design file itself. Commands intentionally left unwrapped (see the plan file): align.muscle is gone in 1.48, sff.multiple crashes, mgcluster needs a blast file, cluster.fit needs a reference set, classify.svm segfaults, make.file relies on file names Galaxy does not keep, translate.seqs is only parsable under its misspelled name, and the session commands (get/set.current, set.dir, set.logfile, set.seed, rename.file, quit, system, help) have no meaning inside Galaxy. 323/323 tests pass in the local Galaxy-faithful harness, planemo lint --fail_level warn exits 0.
The batched lint/md5 clean-up was validated with a local Cheetah render
harness, which is a partial model of Galaxy. Running the suite through
planemo for real uncovered 35 problems that the harness could not see.
This commit fixes all of them; `planemo test tools/mothur/` now executes
323/323 tests successfully and `planemo lint --fail_level warn` is clean.
Tool wrapper bugs:
- cluster.split: `<filter>` expressions evaluated a boolean parameter
with `== "true"`, which is never true (Galaxy hands filters real
bools), so the outputs were silently dropped. Use plain truthiness.
- make.shared, get.oturep: filters accessed conditional values with
attribute syntax (`cond.param`), which raises inside the filter; a
raising filter fails *open*, so unwanted outputs were created. Use
`.get('param')` instead.
- get.lineage, remove.lineage: `$re.sub(..., $taxon)` failed because
text parameters reach Cheetah as InputValueWrapper objects, not
strings. Wrap them in `str()`. Both parameters also needed a
sanitizer that keeps `;`, otherwise Galaxy rewrites the taxonomy
separator to `X` (`Bacteria;Firmicutes;` -> `BacteriaXFirmicutesX`).
- align.check: the sorted map has to be handed to mothur as `map.dat`.
- sens.spec: rewritten command (the matrix branch referenced a
conditional parameter that is not defined in the phylip branch) plus
results-matrix handling and three new tests.
- chimera.slayer: mothur writes no count table when it finds no
chimeras, and Galaxy's mothur.count_table datatype crashes on an
empty file, which turned the whole job into an error. Fall back to
the (unchanged) input count table in that case.
Test/data fixes:
- ftype declarations corrected (pcoa, make.group, count.groups), the
explicit make.fastq format rules moved ahead of the `qfile.ext`
rules (change_format is first-match-wins), and collection element
order fixed for chimera.check, split.groups and list.otulabels.
- make.contigs: expected compressed count-table header.
- sub.sample: the removed name-file input was still referenced; the
count table is the supported path now. Two new tests use
abrecovery data with group columns.
- tree.shared: shared-file test now uses subsampling, because 1.48.5
segfaults (after writing the trees) for single calculators without
it. Documented in the help.
- get.oturep: `groups=` is broken upstream when a group file is also
given, so the test drops it; the *rep.names output needs a name file
even though `sorted=name` alone does not imply one.
- make.lookup: the flow and error-summary fixtures came from different
sequencing runs, which 1.48 rejects. Added a derived, same-run
`testflow.error.summary` fixture and documented the requirement.
- mimarks.attributes: mothur 1.48.6 never registers the `source`
output type, so the command always exits 1 with a known pair of
errors. Keep the wrapper (the bug is upstream and fixed on master)
with a dedicated <stdio> block that only tolerates those two lines.
Cosmetic only (Galaxy parses the attributes by name), but it puts the identifying attributes first and the shared `@PROFILE@` last, matching the usual IUC layout. All 132 wrappers touched; XML still parses and `planemo lint --fail_level warn` is clean.
…oject#5657 and galaxyproject#3663 - chimera.uchime: the count output looked for "template.*uchime.pick.count_table", a name mothur never writes. With the count table symlinked as template.count.dat the command emits template.count.denovo.uchime.count_table, and for a count table without group columns plus dereplicate=T it emits no table at all, so Galaxy always received an empty mothur.count_table and the job died at metadata time (IndexError in CountTable.set_meta, tools-iuc#5178). Fix the glob, pass the input table through when mothur writes none, correct the misleading help text and cover both cases with two new tests. The Galaxy side of galaxyproject#5178 is fixed in galaxy#24840. - make.shared: do not additionally move mothur's own logfile onto the logfile dataset, which is already declared via from_work_dir="mothur.out.log". Galaxy tolerates the double declaration, Pulsar rejects it (tools-iuc#5657). All other wrappers were migrated to "tee mothur.out.log" earlier. - cluster.split, dist.seqs: document that the fasta must be aligned and all sequences of equal length and that mothur may die without a message (exit 139) otherwise (tools-iuc#3663). The hang the issue reports is not reproducible with 1.48, and the "Galaxy ignores the crash" part was fixed by "set -o pipefail" plus <exit_code range="1:" level="fatal"/> long before the issue was filed. This also closes tools-iuc#4413 (detect OOM): mothur's SIGBUS/SIGKILL/SIGSEGV exit codes 135/137/139 now reach the job because of the pipefail option.
github-project-automation
Bot
moved this to Triage/Discuss
in Galaxy Tool Developers SIG Board
This was referenced Nov 11, 2026
Closed
bernt-matthias
commented
Nov 11, 2026
Only cluster (for the agc/dgc methods), cluster.split and chimera.vsearch call
the vsearch executable, so they added it next to the shared `requirements`
macro and the suite ended up needing two environments: mothur alone for 128
tools and mothur+vsearch for 3.
Move the pin into the macro (as @VSEARCH_VERSION@, next to @TOOL_VERSION@) so
the whole suite resolves to one image:
mulled-v2-7796e107c35e50a18912f4feb054b112198e9ab8:705d490ec92a45a8d6be34ea821ba030b92d1c91
(= mothur 1.48.6 + vsearch 2.31.0, the container the vsearch tools already
used in CI)
An instance therefore needs exactly one container / conda environment for the
suite instead of two. taxonomy-to-krona is the only tool without requirements:
it is a coreutils shell script that never runs mothur, so giving it the mothur
image would only add a download.
Note for the reviewer: that mulled image is not yet in quay.io/biocontainers
(only the 1.39.5 + vsearch 2.6.0 one is); Galaxy's `build_mulled` resolver
produces it locally, which is what CI does, and the biocontainers build is
triggered from the requirements once the tool is deployed.
Checks: 131 wrappers still lint clean (`planemo lint --fail_level warn` -> 0)
and resolve to that one image (galaxy-tool-util requirement parsing); the six
touched-or-representative tools pass their 21 tests.
make.lookup: the warning explained how mothur walks the two files in parallel, which column count the error summary has and which value marks a chimera in `numparents`. What a user needs is the one sentence that decides whether the tool works: both files have to come from the same run, so use the error summary that `trim.flows` wrote for this flow file. mimarks.attributes: same for the known-bug warning - the missing `outputTypes["source"] = tempOutNames;` registration and the constructor are relevant when talking to upstream, not in the help page. Kept the observable part (mothur's error message, no `*.source` file, since which version, the mothur <= 1.43 workaround). No behavioural change: 3/3 tests of the two tools still pass, lint clean.
Every mothur release since 1.44 aborts this command: the refactoring in
1e6af2232 ("Explicitly call parent command") dropped
vector<string> tempOutNames;
outputTypes["source"] = tempOutNames;
from the command, and Command::getOutputFileName() refuses to hand out a
filename for a type that is not registered. The command therefore dies with
"[ERROR]: this command doesn't create a source output file." / "[ERROR]: Could
not open" and writes nothing, which is why the previous commits downgraded the
stdio rules and asserted the failure in the logfile.
Instead of shipping a tool that cannot work, pin it to 1.43.0 - the last
release with the registration - and restore the normal error handling and a
real assertion on the output. Verified with mothur 1.43.0: the command exits
0 and writes a 24 line biosample_in.source, byte identical to what a 1.48.6
build with the one line patch back produces.
The help now also says what the output actually is (C++ code that mothur's own
get.mimarkspackage implementation uses, i.e. a developer helper rather than an
analysis step) and why the pin exists. This is the only tool of the suite that
does not run 1.48.6.
The tool runs mothur 1.43.0 (see the previous commit), so hardcode that as the tool version instead of inheriting @TOOL_VERSION@ from the suite. This keeps the version the tool advertises in sync with the binary it actually runs, while the rest of the suite stays on 1.48.6. Also link the fix that is now proposed upstream: mothur/mothur#875 - once a mothur release contains it, the pin can be dropped and this tool can move back to @TOOL_VERSION@. Checks: planemo test 1/1 pass, lint and lint --urls clean (the new link resolves).
…ping commas
Every wrapper used to write its arguments out by hand and then filter the
rendered command through
| sed 's/,[[:space:]]*)/)/' ## a trailing comma segfaults mothur
That guard was dead code in most wrappers: only the ones whose last argument
was inside a Cheetah conditional could ever produce a trailing comma, and for
the rest it merely hid the fact that several booleans carried the comma (or the
whole `key=value` text) inside their truevalue/falsevalue.
mothur is in fact more sensitive here than the guard suggested: a trailing
comma (`,)`) or an empty option (`,,`) makes Utils::splitAtComma index an empty
string and mothur dies with SIGSEGV (exit 139), so a guard that only rewrites
`,[[:space:]]*)` does not protect against empty arguments at all.
Collect the arguments in a Cheetah list and join them instead:
#set $mothur_args = []
#silent $mothur_args.append("fasta=fasta.dat")
#if $calc:
#silent $mothur_args.append("calc=" + str($calc))
#end if
echo 'dist.seqs(${",".join($mothur_args)})'
| mothur
A trailing comma is now impossible by construction, argument order in the
template is preserved, and every appended fragment is a real Python expression
(a plain "$var" inside a #set/#silent literal is not interpolated by Cheetah).
Six booleans used to carry their own separators and are converted to plain
booleans whose parameter text is emitted conditionally: countends
(pairwise.seqs), keepprimer and keepdots (pcr.seqs), jumble
(rarefaction.shared), dups (remove.seqs) and sfftxt (sffinfo). The affected
tests pass true/false, as Galaxy coerces an empty test value to false.
The rendered commands are otherwise token-for-token identical to the previous
ones (verified for all 133 wrappers and all 326 tests), and the whole suite
still passes: 326/326.
bernt-matthias
added
the
skip-url-check
The comments that explain why a wrapper does something non-obvious cited mothur's own source files (`aligncommand.cpp:182`, `clustercommand.cpp`, the `printTable` call in `chimerauchimecommand.cpp`, ...). Those references help a reviewer once, then rot with every mothur release, and three of them had leaked into user-visible help text (`get.lineage`, `get.oturep`, `sub.sample`). Keep the behaviour description - what mothur does, which choices it offers, which output name it writes - and drop the file/line pointers. In the two places where the mothur error message was the real evidence (`indicator`'s "x is not a group in your shared or relabund file") the quoted message stays, without the source location. No command, parameter or assertion changed: the 12 touched tools still pass (50/50 tests) and `planemo lint --fail_level warn` reports nothing, including the RST help checks for the three edited help sections.
bernt-matthias
force-pushed
the
mothur/1.48.6-upgrade
branch
from
November 13, 2026 08:57
3c63333 to
96b61a3
Compare
mothur 1.47.0 removed its BLAST dependency, so the conda package no longer ships blastn and no command has a blast code path left. Three leftovers: * align.seqs and classify.seqs linked the whole conda bin directory into the job directory and then called ./mothur, so that mothur would find blastn next to its own executable. That is dead weight (and pollutes the job directory with ~hundreds of symlinks); mothur finds helper binaries on PATH anyway, and neither command shells out to anything now. * classify.seqs still offered method=knn with search=blast, and pairwise.seqs still offered align=blast. Both are invalid values that mothur does not reject: it substitutes needleman (pairwise.seqs) or builds a plain 8-mer kmer database (classify.seqs), warns on stdout and exits 0, so the run silently produces different results than any advertised algorithm. Both tools now document the removal in their help.
The help texts of 39 wrappers carried "v1.20.0: Updated to Mothur 1.33, added ... parameter" style lines, going back to the 2013 wrapper rewrite, plus two added by this branch. A tool form is not a changelog: the choice today is what the form offers, and a list of parameters that were added in mothur 1.28/1.31/1.33 tells a user nothing about how to run the tool. Removed all 41 lines; nothing else in the help changes.
The command emitted set.dir(mothurfiles=.) twice in the same subshell; the first call is enough to keep mothur's default-files path non-empty.
…p texts
The four "**Changes in mothur 1.48**" sections and three sentences that said when a
behaviour changed ('Since mothur 1.46 ...', 'As of mothur 1.48 the label parameter was
replaced by cutoff', 'as of mothur 1.48 a group file can no longer be sorted') describe
things a user needs today: which files are written, that a count table is required, and
that some options are broken upstream. They are now stated in the present tense under a
neutral '**Notes**' heading, without asking the reader to reconstruct the history.
bernt-matthias
force-pushed
the
mothur/1.48.6-upgrade
branch
from
November 13, 2026 09:10
96b61a3 to
e25b4c4
Compare
The help warned that an empty mothur.count_table used to abort the job at metadata time (galaxy#24840) and pointed at tools-iuc#5178. That is an internal detail of the Galaxy datatype and none of the user's business; the wrapper never produces the empty file, as the section above it already states. The changelog line 'Version 1.23.0: Upgrades tool dependency to mothur 1.33 ...' goes with it. The command keeps its pass-through fallback and the comment explaining it.
The note explained that the .file summary lists temporary files by their name in the job's working directory and therefore cannot be fed back into the tool. There is no input that would accept it, so the note only raised a question nobody asked; the sentence listing the outputs of 'perform clustering? = off' stays.
Eleven help texts ended in 'For calc parameter choices see: https://mothur.org/wiki/calculators/' - a bare URL next to prose that already linked calculators properly (collect.shared even defined the target and then did not use it). They now say 'see calculators_.' and define the target; seq.error's MiSeq SOP pointer, shhh.flows' lookup files and chimera.uchime's link to uchime's original home are references too. No bare URL is left inside a <help> block.
- dist.seqs: "mothur releases before 1.48 also printed that message when the lengths differed but then crashed with std::bad_alloc or kept running" - what 1.46 did is not what the user needs to know when the job fails today. - get.lineage: the "Note on mothur 1.48" section explained how the wrapper renames mothur's [filename],pick.[extension] output; that is the wrapper's business, the command keeps the comment. - get.seqs: the last surviving "v.1.27.0 : Updated to Mothur 1.33" changelog line.
… broke it 'does not work upstream' asks the reader to know what upstream means; the sentence already says what happens (mothur aborts with "[ERROR]: <sequence> is not in your count table").
parse.list was dropped from mothur in v1.43.0 (1.42.1 is the last release that knows the command), so the wrapper is archived with the pre-upgrade copy of macros.xml, which pins mothur 1.39.5 - the version it was written and last tested against. deprecated/ is not linted or tested by CI.
make.design disappeared from the suite with the 1.48.6 upgrade because mothur has no command of that name - it never had one, the wrapper writes the design file itself. But its output was the old two column format without a header, which mothur rejects with "Expected 'group' and found '<first group>'" since the design file parser requires a header row, and which cannot express more than one grouping at all. The wrapper now writes a design table with a "group" header, one line per group of the source file and one set column per grouping, refuses names with whitespace (which would break the table) and refuses to write a table whose columns do not cover every group of the source file. Tests cover a shared and a group file source, two groupings and the missing group error.
Without group information mothur writes a single map file and names it after the file it was handed, so the only element of the precluster.map collection was called "fasta" -- the name of the symlink created for the job, not the name of the dataset the user picked. Rename the map file after the element identifier of the input dataset in that case (groups keep their names, since mothur already names those after the group). The two tests now check for the renamed element. The help text also promised a ``*.precluster.count_table`` dataset, which does not exist under that name: the output is called precluster.count_table in the tool form. Say that instead, and describe the two shapes of the map collection.
The help texts talked about files where they meant the datasets a user picks in the form and the datasets the tool writes. Say dataset for those, and keep "file" only where it is the name of a mothur format (fasta, name, group, shared, design, flow, ...) or part of a quoted mothur message. Where a help text spelled out an internal file name of the job directory (``*.trim.count_table``, ``count.subsample.count_table``, ``*.rep.count_table``, ``.uchime.count_table``, ``*.source``) it now names the dataset or collection the user actually sees in the history.
The notes talked about "Mothur 1.46+" and "reproducible with mothur 1.46 too". The version is of no use to the user: the wrapper ships one mothur and the behaviour they have to work around is the one in front of them. State the restriction without dating it.
The help texts described results with mothur's file name patterns
(".chop.fasta", ".indicator.summary", ".shared.seqs", ".otu", "tax.summary
files"). Those only reproduce the name of the file in the job directory
and tell the user nothing about where to look in the history.
Refer to the datasets by the label they carry in the history instead --
chop.fasta, indicator.summary/indicator.tre, shared.seqs/shared.accnos/
shared.fastas, relabund, the "OTUs per label" collection, shared/
cons.taxonomy/tax.summary, database.
pre.cluster also explained that no file named precluster.names is written;
it now says that the sequence names are not returned separately, which is
what the user cares about.
merge.taxsummary's output was labelled "merged sff", a leftover from
merge.sfffiles; it is a merged tax.summary.
Galaxy falls back to the argument when a parameter has no name (galaxy/tool_util/parser/util.py: _parse_name strips leading dashes and turns the remaining dashes into underscores), so for a plain identifier the two spellings denote the same parameter and nothing else has to change: the Cheetah variables, the test parameters, the filter refs and the conditional `when` keys all keep their old spelling. 786 parameters were spelled with `name` and 295 with `argument`; the whole suite now uses `argument` (1081 parameters). The 13 parameters that carry both keep both: there the argument is the keyword mothur expects (list, shared, column, phylip, fasta, reference, search) and the name is the Galaxy side identifier that is used in more than one `when` branch. Conditionals, sections and repeats keep their name - there is no argument attribute for them.
bgruening
reviewed
Nov 13, 2026
…icrobiome output to mothur These three parameters were offered in the tool interface but never made it into the mothur command line. The new tests assert on the command line that mothur echoes into its logfile, so the options have to stay wired.
Without pipefail only the exit status of the last sed reached Galaxy, so a failing stage produced an empty dataset with a successful job.
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.
FOR CONTRIBUTOR:
There are two labels that allow to ignore specific (false positive) tool linter errors:
skip-version-check: Use it if only a subset of the tools has been updated in a suite.skip-url-check: Use it if github CI sees 403 errors, but the URLs work.To request a review once your PR is ready, comment "please review" on the PR. This will
automatically apply the
ready-for-reviewlabel if the PR is not a draft, all reviewthreads are resolved, and all CI checks have passed.