-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add FastQC to Optimus and SmartSeq2 #202
base: master
Are you sure you want to change the base?
Conversation
retest this please |
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.
LGTM
} | ||
|
||
File barcoded_bam = select_first([AttachBarcodes.bam_output, AttachBarcodesNoIndex.bam_output]) | ||
Array[File] fastqc_output_htmls = select_first([FastQC.fastqc_htmls,FastQCNoIndex.fastqc_htmls]) |
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 you are expecting multiple zips or htmls then I think select_all()
would be better here.
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.
Since only one of FastQC
and FastQCNoIndex
will ever be run, using select_all()
would just result in an extra unnecessary level of array.
pipelines/optimus/Optimus.wdl
Outdated
@@ -222,5 +243,9 @@ workflow Optimus { | |||
|
|||
# zarr | |||
Array[File]? zarr_output_files = OptimusZarrConversion.zarr_output_files | |||
|
|||
# fastqc | |||
Array[Array[File]] fastqc_htmls = fastqc_output_htmls |
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.
Unless you really want Array[Array[File]]
here it seems like you'd be better off with flatten(fastqc_output_htmls)
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.
agreed, will do.
test/optimus/pr/ValidateOptimus.wdl
Outdated
@@ -51,6 +52,14 @@ task ValidateOptimus { | |||
fail=true | |||
fi | |||
|
|||
for htmlfile in ${sep=' ' fastqc_htmls}; do |
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 guess you can't easily check the zips here? The test could at least assert that the number of zip files generated is correct.
@kbergin do we want to finish work on this? |
I've made a ticket for us to rebase this and get these changes reviewed and merged in. |
Purpose
QA Inputs in SS2 and Optimus (and any future pipeline using fastqs as input)
Changes
Added FastQC task. For increased speed FastQC is run on only a subset of the fastq reads (can be set by inputs to task).