Add bcalm flowcraft #149
Add bcalm flowcraft #149
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #149 +/- ##
==========================================
+ Coverage 43.48% 43.55% +0.06%
==========================================
Files 63 63
Lines 5889 5896 +7
==========================================
+ Hits 2561 2568 +7
Misses 3328 3328
Continue to review full report at Codecov.
|
tiagofilipe12
left a comment
There was a problem hiding this comment.
Hi @natir ! Welcome to FlowCraft! I made some suggestions to your PR but I didn't test the module, just checked the code. Maybe @ODiogoSilva also has some suggestions!
Also please add this new component to changelog.md so we can track the changes made to the next release of FlowCraft.
flowcraft/generator/engine.py
Outdated
| "seq_typing": typing.SeqTyping, | ||
| "sistr": typing.Sistr, | ||
| "skesa": assembly.Skesa, | ||
| "bcalm": assembly.Bcalm, |
There was a problem hiding this comment.
Although this isn't required at all, for code reading it is easier to maintain the alphabetical order. Can you please change this to keep the alphabetical order of the process_map dict?
| "cpus": 4, | ||
| "memory": "{ 5.GB * task.attempt }", | ||
| "container": "quay.io/biocontainers/quast", | ||
| "version": "2.2.0", |
There was a problem hiding this comment.
are you sure that this is the version of the container that you want to build? I couldn't find it in here.
There was a problem hiding this comment.
The Docker image doesn't exist yet, but I think it's being built automatically.
https://quay.io/repository/biocontainers/bcalm
The version number will be different. You'll have to get it once the build completes from
https://quay.io/repository/biocontainers/bcalm?tab=tags
There was a problem hiding this comment.
When image are build I fix this issue.
| @@ -0,0 +1,30 @@ | |||
| # Check parameter | |||
There was a problem hiding this comment.
Comments in groovy language use // instead of #. Please edit all the comments to // instead of #.
| val KmerSize from Channel.value(params.bcalmKmerSize{{param_id}}) | ||
|
|
||
| output: | ||
| file "*" |
There was a problem hiding this comment.
Do you require that all the files generated by bcalm are passed to the output channel so it can link with other processes? Or can you narrow them somehow?, for instance like you did for the publishDir.
There was a problem hiding this comment.
Hi @natir , thanks for the really nice contribution and it looks almost completely ready. I've left only a couple of comments, of which the clearInput parameter is the most important.
| self.input_type = "fastq" | ||
| self.output_type = "fasta" | ||
|
|
||
| self.dependencies = ["integrity_coverage"] |
There was a problem hiding this comment.
From what I see in the nextflow template, this dependency is not really required, right?
tiagofilipe12
left a comment
There was a problem hiding this comment.
Seems good to me. Let's just wait for the final tag version of the docker image before merging into dev. Also changelog will be conflicting with the current dev changelog.md. You will need to update your branch so that it can properly merge with the dev. Good work 👍 !
changelog.md
Outdated
|
|
||
| ### New components | ||
| - Added component `fast_ani`. | ||
| - Added component `bcalm`. |
There was a problem hiding this comment.
Hmm this should be on the next release of the dev branch and not under the 1.3.1 release. But to do so you will need to merge the dev branch into your fork so that you can you can resolve the conflicts with dev
|
The |
|
The bcalm image exists now! https://quay.io/repository/biocontainers/bcalm?tab=tags |
ODiogoSilva
left a comment
There was a problem hiding this comment.
Looks good! Thanks for the effort 👍
Hi everyone,
It's my first pull request to flowcraft, so I probably make many mistake, thank for your help.
I add bcalm recently in bioconda, so it's possible the biocontainer image isn't build.