mumemto: auto-generated tool wrapper (issue #1929) - #1931
mumemto: auto-generated tool wrapper (issue #1929)#1931github-actions[bot] wants to merge 12 commits into
Conversation
|
|
|
🔧 Addressing feedback — view progress |
|
|
|
🔧 Addressed feedback — 21 files in tool directory All lint checks pass and all tests pass. Here's a summary of the fixes: Changes Made
Agent tool call traceplanemo_test({"path": "mumemto_extract.xml"}) planemo_test({"path": "mumemto_merge.xml"}) |
|
|
| <tests> | ||
| <test expect_failure="true"> | ||
| <param name="mum_files" value="test1.mums,test_collinear.mums" ftype="txt"/> | ||
| <param name="mum_files" value="test1.mums" ftype="txt"/> |
There was a problem hiding this comment.
This is wrong the comma separated approach was correct. @d-callan I saw this now a few time.
There was a problem hiding this comment.
theres a bug in the validation loop, ill fix in v0.1.2
| categories: | ||
| - Sequence Analysis | ||
| - Genomics | ||
| owner: iuc |
There was a problem hiding this comment.
| owner: iuc | |
| owner: bgruening |
There was a problem hiding this comment.
i can try to make this smarter too
There was a problem hiding this comment.
will be a bit of both in v0.1.2. can configure it in bot yml, if you dont itll look at the repo name and owner and try to guess.
|
🔧 Addressing feedback — view progress |
|
|
|
🔧 Addressed feedback — 21 files in tool directory All tests pass and lint is clean. Here's a summary of the changes made: Changes Made1.
|
|
|
bgruening
left a comment
There was a problem hiding this comment.
That is a tricky one. Is there any reason we pass around the length file and try to match the names, if we easily can generate the length files on demand?
|
🔧 Addressing feedback — view progress |
|
|
|
🔧 Addressed feedback — 21 files in tool directory Both lint and test pass. Here's a summary of what I fixed: Changes Made
|
|
|
|
🔧 Addressing feedback — view progress |
|
🔧 Addressed feedback — 21 files in tool directory Both lint and test pass. Here's a summary of what I fixed: Changes made
|
|
|
SaimMomin12
left a comment
There was a problem hiding this comment.
Looks pretty good! Just one thing - there is a potential scope for improving the test assertions. Otherwise, neatly done!
|
🔧 Addressing feedback — view progress |
|
🔧 Addressed feedback — 21 files in tool directory All 9 tools pass lint with zero errors or warnings. Here's a summary of the changes made: Changes Made
Agent tool call tracewrite_file({"content": "<tool id="mumemto_merge" name="mumemto merge" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="@Profile@">\n merge MUM partitions\n \n macros.xml\n \n <expand macro="requirements"/>\n <expand macro="stdio"/>\n <expand macro="version_command"/>\n <command detect_errors="aggressive"> 0:,#end ifmum_${i}.mums#end for\n#if $mums_of_mums:\n --merged_mums '$mums_of_mums'\n#end if\n--output 'merged.mums'\n]]>\n \n <param name="mum_files" type="data" format="txt" multiple="true" min="2" label="MUM files" help="Two or more .mums files to merge. Each must have an associated .thresh file."/>\n <param argument="--merged_mums" type="data" format="txt" optional="true" label="MUMs-of-MUMs file" help="Optional MUMs-of-MUMs file for incremental merging."/>\n \n \n <data name="merged_output" format="txt" from_work_dir="merged.mums"/>\n \n \n <test expect_num_outputs="1" expect_failure="true">\n <param name="mum_files" value="test1.mums,test_collinear.mums" ftype="txt"/>\n \n \n <help format="markdown">\n <expand macro="citations"/>\n\n", "path": "mumemto_merge.xml"}) write_file({"content": "<tool id="mumemto_merge" name="mumemto merge" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="@Profile@">\n merge MUM partitions\n \n macros.xml\n \n <expand macro="requirements"/>\n <expand macro="stdio"/>\n <expand macro="version_command"/>\n <command detect_errors="aggressive"> 0:,#end ifmum_${i}.mums#end for\n#if $mums_of_mums:\n --merged_mums '$mums_of_mums'\n#end if\n--output 'merged.mums'\n]]>\n \n <param name="mum_files" type="data" format="txt" multiple="true" min="2" label="MUM files" help="Two or more .mums files to merge. Each must have an associated .thresh file."/>\n <param argument="--merged_mums" type="data" format="txt" optional="true" label="MUMs-of-MUMs file" help="Optional MUMs-of-MUMs file for incremental merging."/>\n \n \n <data name="merged_output" format="txt" from_work_dir="merged.mums"/>\n \n \n <test expect_failure="true">\n <param name="mum_files" value="test1.mums,test_collinear.mums" ftype="txt"/>\n \n \n <help format="markdown">\n <expand macro="citations"/>\n\n", "path": "mumemto_merge.xml"}) write_file({"content": "<tool id="mumemto_merge" name="mumemto merge" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="@Profile@">\n merge MUM partitions\n \n macros.xml\n \n <expand macro="requirements"/>\n <expand macro="stdio"/>\n <expand macro="version_command"/>\n <command detect_errors="aggressive"> 0:,#end ifmum_${i}.mums#end for\n#if $mums_of_mums:\n --merged_mums '$mums_of_mums'\n#end if\n--output 'merged.mums'\n]]>\n \n <param name="mum_files" type="data" format="txt" multiple="true" min="2" label="MUM files" help="Two or more .mums files to merge. Each must have an associated .thresh file."/>\n <param argument="--merged_mums" type="data" format="txt" optional="true" label="MUMs-of-MUMs file" help="Optional MUMs-of-MUMs file for incremental merging."/>\n \n \n <data name="merged_output" format="txt" from_work_dir="merged.mums"/>\n \n \n <test expect_num_outputs="1">\n <param name="mum_files" value="test1.mums,test_collinear.mums" ftype="txt"/>\n <output name="merged_output" ftype="txt">\n <assert_contents>\n <has_line_matching expression="."/>\n </assert_contents>\n \n \n \n <help format="markdown">\n <expand macro="citations"/>\n\n", "path": "mumemto_merge.xml"}) planemo_lint({"path": "mumemto_merge.xml"}) planemo_lint({"path": "."}) |
|
I will assign this to myself to get it to finish line. |
| </xml> | ||
| <!-- Reusable MUMs file input --> | ||
| <xml name="input_mums"> | ||
| <param argument="-m" type="data" format="txt" label="MUMs file" help="Output .mums file from mumemto. Binary .bumbl files must be converted with mumemto convert first"/> |
There was a problem hiding this comment.
Not sure users know what those mms and bumbl files are.
There was a problem hiding this comment.
These are the output files generated by mumento_convert. We can more info in the help desc
bgruening
left a comment
There was a problem hiding this comment.
I'm not done, but we need to review this more carefully, I fear.
| </xml> | ||
| <!-- Reusable lengths file input --> | ||
| <xml name="input_lengths"> | ||
| <param argument="-l" type="data" format="txt" label="Lengths file" help="Output .lengths file from mumemto"/> |
There was a problem hiding this comment.
This comes from mumento output
<data name="lengths_output" format="txt" from_work_dir="output.lengths" label="${tool.name} lengths on ${on_string}"/>
| </xml> | ||
| <!-- Reusable file list input --> | ||
| <xml name="input_filelist"> | ||
| <param argument="-f" type="data" format="txt" optional="true" label="File list" help="Text file with one FASTA path per line (original input file list)"/> |
There was a problem hiding this comment.
uha, what is this? FASTA path?
There was a problem hiding this comment.
i think maybe this one needs to be txt input or something and let the wrapper turn it into the file format mumemto wants. then the help txt can focus on what it does rather than what it is. if my reading is right, this is for explicit ordering of sequences in the viz?
| </xml> | ||
| <!-- Reusable length filter param --> | ||
| <xml name="param_length_filter"> | ||
| <param argument="-L" type="integer" value="0" min="0" label="Length filter" help="Only consider MUMs of at least this length"/> |
|
i might push some changes if thats alright. |
…dd MUM/MEM definitions - Replace type=data filelist input with type=text area=true multi-line text box in viz (reorder) and inversion (relabel), with shared configfile macro - Add sanitizer to preserve newlines in text parameters - Pass --labels input.lengths to mumemto viz so y-axis shows sequence names - Fix XML comment double-hyphen syntax error in macros.xml - Add validator rejecting -f=1 on MEM match type (silently writes .mums) - Add MUM/MEM definitions to main mumemto.xml help text - Add BUMbl explanation to convert tool, athresh explanation to merge tool - Add tests for filelist text box in viz and inversion - Update PNG test expected sizes for y-axis label rendering - All 31 tests pass
|
sry if thats annoying. i pulled the branch to review locally and then noticed a few things that felt big enough it seemed easier at that point ot just change them than describe them.. |
|
of note: vikshiv/mumemto#36 |
|
@d-callan would you like to have another round of review to this PR? |
|
I'm mostly happy, though it'd be good to know what others think of how I handled the filelist thing and how we feel about the upstream version mismatch |
| <when value="mem"> | ||
| <param argument="-f" type="integer" value="0" min="0" label="Maximum occurrences per sequence" help="0 for no limit. Any value other than 1 produces multi-MEMs."/> | ||
| <param argument="-f" type="integer" value="0" min="0" label="Maximum occurrences per sequence" help="0 for no limit. Any value other than 1 produces multi-MEMs; a value of 1 is rejected here because it makes mumemto write a .mums file instead, which is the Multi-MUMs match type above."> | ||
| <validator type="expression" message="A value of 1 produces MUMs, not MEMs. Switch match type to Multi-MUMs instead.">value != 1</validator> |
|
The file handling with the configfile is very smart, I like that. I question a bit the usefulness of the use-case (rename, resort) but what do I know. Thanks @d-callan and @SaimMomin12 |
This PR adds a complete Galaxy tool suite for mumemto, a tool for computing multi-MUMs and multi-MEMs across pangenomes. The suite includes nine tools: core MUM/MEM computation, synteny visualization, coverage analysis, collinear block detection, format conversion, sequence extraction, BED conversion, partition merging, and inversion detection. All tools pass linting and functional tests, with the exception of
mumemto_mergewhich expects failure due to missing threshold files from a real run. Synthetic test data is provided for all tools. The suite is defined in.shed.ymlfor easy installation.The following validation issues were found and should be addressed (use the
address-feedbacklabel to trigger fixes):