-
Notifications
You must be signed in to change notification settings - Fork 888
Adding cnaqc tool module #8890
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: master
Are you sure you want to change the base?
Adding cnaqc tool module #8890
Conversation
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 changing to a clean branch :) I added some suggestions!
@@ -0,0 +1,46 @@ | |||
process CNAQC { | |||
tag "$meta.id" | |||
label 'process_low' |
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.
Does your script allow multi-threading? If no then change to process_single :)
@@ -0,0 +1,46 @@ | |||
process CNAQC { | |||
tag "$meta.id" |
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.
tag "$meta.id" | |
tag "${meta.id}" |
|
||
output: | ||
tuple val(meta), path("*_qc.rds"), emit: qc_rds | ||
tuple val(meta), path("*_data_plot.rds"), path("*_qc_plot.rds"), emit: plot_rds |
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.
Can you adjust this to have only one file per output channel?
stub: | ||
def args = task.ext.args ?: '' | ||
def prefix = task.ext.prefix ?: "${meta.id}" | ||
|
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.
licence: ["GPL v3-or-later"] | ||
identifier: "" | ||
input: | ||
# Only when we have meta |
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.
# Only when we have meta |
type: file | ||
description: R object with mutation data | ||
pattern: "*.rds" | ||
ontologies: [] |
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.
Can you look for ontologies please?
ext.args = [ | ||
genome : "GRCh38", | ||
matching_strategy : "rightmost", | ||
purity_error : "0.05", | ||
muts_per_karyotype : "25", | ||
min_absolute_karyotype_mutations : "100" |
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.
Are these necessary arguments for the tool to be able to run? If yes they need to be reflected in the module definition as value input channels for example :)
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 am not an expert on R so maybe someone else needs to be asked to review this part!
{ assert snapshot( | ||
process.out.versions | ||
).match() }, | ||
// Check nondeterministic files exist | ||
{ assert file(process.out.qc_rds[0][1]).exists() }, | ||
{ assert file(process.out.plot_rds[0][1]).exists() }, | ||
{ assert file(process.out.plot_pdf_data[0][1]).exists() }, | ||
{ assert file(process.out.plot_pdf_qc[0][1]).exists() } |
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.
Can we add more stuff to the snapshot? Maybe at least get the name of each file that is in th eoutput and append that to the snap?
then { | ||
assertAll( | ||
{ assert process.success }, | ||
{ assert snapshot(process.out).match() } |
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.
Can you add path(process.out.versions[0]).yaml to check whether the version is accessed correctly?
PR checklist
Closes #8610
Reopening the PR with a clean state as asked in #8714 (comment)
versions.yml
file.label
nf-core modules test <MODULE> --profile docker
nf-core modules test <MODULE> --profile singularity
nf-core modules test <MODULE> --profile conda
nf-core subworkflows test <SUBWORKFLOW> --profile docker
nf-core subworkflows test <SUBWORKFLOW> --profile singularity
nf-core subworkflows test <SUBWORKFLOW> --profile conda