feat(galaxy): add tool wrappers for all five subcommands - #18
Merged
Conversation
Five Galaxy tools, one per subcommand, sharing a macros.xml. The layout matches
galaxyproject/tools-iuc, so submitting there is a directory copy rather than a
rewrite.
pathotypr is already on Bioconda with a BioContainer built from it and is
registered on bio.tools, so the wrappers declare the conda package and nothing
is vendored.
Verified with planemo: lint is clean on all five with no warnings, and all 8
tests pass. The test suite was itself checked by mutation: changing the output
prefix so it no longer matches from_work_dir fails both classify tests, which is
how we know the assertions read the real outputs.
The test data is 64 KB of synthetic input, generated deterministically by a
script kept alongside it. It is not filler. The generator asserts that every
marker k-mer occurs exactly once in the reference, in both orientations, because
a marker matching in two places would make the tests pass for the wrong reason.
The expected calls follow from how the samples are built, and they cross-check
each other: split-fastq on a sample's reads produces the same summary as
classify on its assembly, which is the property the marker format promises.
Three things had to be worked around, none of which is visible from the CLI
help and all of which were found by running the tools:
* classify and split-fastq take the sample name from the input filename.
Galaxy datasets arrive as dataset_NNN.dat, so outputs would have been named
after a Galaxy id. Inputs are symlinked to stable names first.
* match writes the absolute paths of its inputs into the report's first
column, which would differ between instances. Same fix.
* --min-alt-percent is parsed as an integer. A Galaxy float parameter renders
95.0, which the tool rejects with "invalid digit found in string". It is
declared as an integer, which also means fractional thresholds such as
99.5% are not reachable.
Marker panels and models come from the history for now. A data manager and a
.loc table would let an administrator install the published MTBC panels once,
which is how Galaxy usually handles reference data; that is left as its own
piece of work rather than half-built.
labeler.yml was written before galaxy/ existed and has no catch-all, so the pull request adding five Galaxy wrappers arrived with no labels at all. The list has now aged exactly the way the comment in that file predicts a file-by-file list would.
…output Checked against a current tools-iuc tool rather than from memory, which turned up three things. .shed.yml was missing name, type and auto_tool_repositories. That last one is what turns a directory of five tools into five ToolShed repositories, so without it the suite would not have been built the way the IUC expects. Its remote_repository_url also pointed at this repository rather than at the path the wrappers will live at inside tools-iuc. The masked FASTA output had no test. It is a collection discovered by pattern, so its naming is exactly the kind of thing that breaks silently. Verified first that the file is sample1_masked.fasta, that the discovered designation is sample1, and that it carries exactly eight Ns, one per marker. The input genomes contain no N at all, so an N in the output can only have come from masking. detect_errors stays exit_code rather than the aggressive setting the reference tool uses. That is now a measured decision: pathotypr writes its INFO log to stderr, and aggressive scans stderr, so a log line containing the word error would fail an otherwise correct job. No EDAM annotations. The reference tool carries none either, and wrong ontology terms are worse than absent ones. planemo: lint clean on all five, 9 of 9 tests passing.
Member
Author
Verified against the real Bioconda packageThe one gap in the earlier verification is now closed. All 9 tests passed. Provenance of the binary the jobs actually ran, read from the environment planemo built:
The job log shows that environment being activated, and contains zero references to the local build directory, so nothing fell back to a binary that happened to be on PATH. This exercises the whole chain the IUC CI will exercise: the |
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.
Five Galaxy tools, one per subcommand, ready to submit to
galaxyproject/tools-iuc.Where this sits
Three of the four prerequisites were already met, which is why this is mostly wrapper work:
quay.io/biocontainers/pathotypr:1.0.2--hef77e2b_0pathotyprNothing is vendored; the wrappers declare the conda package.
Verified
planemo lintplanemo testfrom_work_dirfails bothclassifytestsThat last line is the one that matters: it is how we know the assertions read the real outputs rather than passing because nothing was checked.
Test data: 64 KB, and not filler
A 2 kb synthetic reference, eight markers, two samples, paired reads at about 18x, and a twelve-genome training set in two classes. Generated deterministically by a script kept next to it.
The generator asserts that every marker k-mer occurs exactly once in the reference, in both orientations. A marker matching in two places would make the tests pass for the wrong reason.
The expected calls follow from how the samples were built, and they cross-check each other:
sample1carries L2 to L2.1 to L2.1.1 plus a RIF marker, soclassifycalls L2 and reportsrpoB/S450Lsplit-fastqon the same sample's reads produces the same summary asclassifyon its assembly, which is exactly the property the marker format promisespredictassigns the two held-out queries to L2 and L3matchpicksref_A, which issample1's own genomeThree things that had to be worked around
None of these is visible in the CLI help. All three were found by running the tools.
classifyandsplit-fastqtake the sample name from the input filename. Galaxy datasets arrive asdataset_NNN.dat, so the outputs would have been named after a Galaxy id. Inputs are symlinked to stable names first.matchwrites the absolute paths of its inputs into the first column of its report. Those differ between instances, so the same fix applies.--min-alt-percentis parsed as an integer. A Galaxyfloatparameter renders95.0, which the tool rejects withinvalid digit found in string. It is declared as an integer here.That third one is worth a second look outside this pull request:
min_alt_percent: u32means a threshold of 99.5% cannot be expressed, even though the comparison itself is done inf32. For separating clonal samples from mixtures, the step from 99 to 100 is a coarse one. Nothing here depends on changing it, and the documentation does not claim otherwise, so it is recorded rather than acted on.Not included, on purpose
Marker panels and models come from the history. A data manager plus a
.loctable would let an administrator install the published MTBC panels once so users pick them from a dropdown, which is how Galaxy normally handles reference data. Shipping a data table with nothing to populate it would move the burden to administrators without helping anyone, so it is left as its own piece of work.Submitting
The layout matches tools-iuc, so submission is
cp -r galaxy/tools/pathotypr <tools-iuc>/tools/, then a pull request there. Their CI runs the same lint and tests on Linux, where the Bioconda package resolves.Worth noting for that pull request: TB-Profiler is already in Galaxy, but fastlin is not, so there is currently no alignment-free MTBC lineage typer on the platform.