-
Notifications
You must be signed in to change notification settings - Fork 10
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
DATAOPS-716 Use customized list of adapters, FastQC #60
DATAOPS-716 Use customized list of adapters, FastQC #60
Conversation
When hoovering over the adapter content plot in the the FastQC output displayed in the end of all seqreports the name of the adapter found is appearing. The adapters and names used are by default the ones specified in the FastQC software. However, the provided list is outdated and many of the adapters are used in several library preparation kits. A customized list can now be used as input instead to look for adapters we use at SNP&SEQ and to display relevant names when found. Files changed: - New file, adapter_list_fastqc.txt, added in config/tool_config/ to specify the adapters to look for. - The fastqc command in main.nf is now updated with the additional a-flag that is used to specify the use of the file described under the previous point.
I did not create any new tests for this feature. I just added the new argument, ran all the already available tests (unit+integration) and inspected the reports manually to make sure that nothing fails and reports are updated. |
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.
Great work, looks great! Added some minor suggestions.
I think it is fine to not add any specific tests for this case, the integration test covers testing that the new parameter does not break the pipeline and that a FASTQC section has been added to the report.
@@ -0,0 +1,31 @@ | |||
# This file is copied and modified from | |||
# https://github.com/s-andrews/FastQC/blob/master/Configuration/adapter_list.txt |
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.
For traceability it could be better to use the permalink, e.g. https://github.com/s-andrews/FastQC/blob/1faeea0412093224d7f6a07f777fad60a5650795/Configuration/adapter_list.txt
This might be a bit overkill though, let me know what you think.
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 think that is a good idea since I made a copy of this file in its current state and it might change.
main.nf
Outdated
@@ -135,7 +135,8 @@ workflow CHECK_RUN_QUALITY { | |||
GET_QC_THRESHOLDS(run_folder) | |||
GET_METADATA(run_folder) | |||
project_and_reads = get_project_and_reads(params.run_folder) | |||
FASTQC(project_and_reads) | |||
FASTQC(project_and_reads, | |||
params.config_dir) | |||
FASTQ_SCREEN(project_and_reads, | |||
params.config_dir, |
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.
While at it, could you update the indentation here so it follows the same structure?
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.
Yes!
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.
This is a bit weird though. When I open it in VS Code the indentation looks correct.
Co-authored-by: Matilda Åslin <matilda.aslin@medsci.uu.se>
Co-authored-by: Matilda Åslin <matilda.aslin@medsci.uu.se>
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.
Great!
When hoovering over the adapter content plot in the the FastQC output displayed in the end of all seqreports the name of the adapter found is appearing. The adapters and names used are by default the ones specified in the FastQC software. However, the provided list is outdated and many of the adapters are used in several library preparation kits. A customized list can now be used as input instead to look for adapters we use at SNP&SEQ and to display relevant names when found.
Files changed: