-
Notifications
You must be signed in to change notification settings - Fork 484
autocycler suit #7247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
autocycler suit #7247
Conversation
|
@bernt-matthias Can you review this autocycler suite and merge it when you can. Many thanks |
|
@bernt-matthias Thank you for reviewing the tools . I am working on the improvements at the moment and will resubmit them when I am done. |
|
@mthang please ping us when you think a new review is needed :) Thanks a lot this looks great! |
|
@bgruening @bernt-matthias It took me a while to fix the wrapper and the test-data. Can you please review the wrappers again. Many thanks. |
|
The shed.yml was for some reason broken, tests are now running. Lets see what happens :) Thanks a lot @mthang |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your efforts. I added some more comments. Some apply for many/all tools.
| <test> | ||
| <param name="in_gfa" value="consensus_assembly_unfiltered.gfa"/> | ||
| <param name="remove" value="743,782"/> | ||
| <output name="cleaned_gfa" file="clean.gfa" ftype="gfa1"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having file and assert_contents seems redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the assert_contents and keep the file only. Should I remove all assert_contents if I use "file" in my test_unit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If an assertion captures the essence of the test I would even prefer the assertion over the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the assertion back to the autocycler_clean.xml to ensure the test works. The planemo failed after I removed the file="clean.gfa" ftype="gfa1"
| </inputs> | ||
| <outputs> | ||
| <collection name="subsampled_reads" type="list" label="${tool.name} on ${on_string}: subsampled reads"> | ||
| <discover_datasets pattern="(?P<designation>.+)\.fastq" directory="out_dir" format="fastq" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
infer format from input?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like so: ae231f6 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for providing the changes ! I was puzzled what "infer format from input" means in Galaxy. I understand now. As you probably know, the autocycler subsample produces uncompressed fastq files instead of compressed fastq.gz. That's the reason I didnt update the test section. Anyway, thanks a lot!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the efforts. This looks nearly ready to go :)
if there is only one output
and split yaml output in separate output
|
Hi @mthang I took some time to rewrite quite a bit to have a more Galaxy look-and-feel, i.e. mainly I removed the nested collectiond and split collections containing fasta and gfa. What do you think? |
ff0f05e to
6d621fc
Compare
| <inputs> | ||
| <param argument="--reads" type="data" format="fastqsanger,fastqsanger.gz" label="Input long reads (FASTQ format)"/> | ||
| <param name="assembler" type="select" label="Tool"> | ||
| <option value="genome_size">genome_size</option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking if we should only support the genome_size command?
We have tools for most assemblers, except for metamdbg, plassembler, lja, and myloasm.
This would also reduce the number of requirements.
FOR CONTRIBUTOR:
This is a collection of tools for Autocycler - https://github.com/rrwick/Autocycler .