Skip to content
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

Initial commit to add fq lint module #1453

Closed
wants to merge 1 commit into from

Conversation

oligomyeggo
Copy link

Addition to the RNA-seq pipeline to include the fq lint module and validate single-end or paired-end FastQ files. This will prevent the pipeline from continuing and running more computationally-heavy steps on corrupted FastQ files, and will address open issue #1281 .

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/rnaseq branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

Copy link
Member

@pinin4fjords pinin4fjords left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, this will be a great contribution!

But any 'nf-core' components (modules, subworkflows) need to be updated in the modules repo first (e.g. here). Then we can use the nf-core tooling to update those components in the subworkflow.

It would also be good to capture the outputs. So the subworkflow should have an output channel for the linting report, and we should set a publishing rule to place the output in the output directory. I can help with that when we get there.

@@ -4,6 +4,7 @@ include { BBMAP_BBSPLIT } from '../../../modules/nf-core/bbmap
include { CAT_FASTQ } from '../../../modules/nf-core/cat/fastq/main'
include { SORTMERNA } from '../../../modules/nf-core/sortmerna/main'
include { SORTMERNA as SORTMERNA_INDEX } from '../../../modules/nf-core/sortmerna/main'
include { FQ_LINT } from '../../../modules/nf-core/fq/lint/main'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this change has not been made on nf-core/modules- it needs to be done there, and then the component added here

Comment on lines +119 to +134
ch_reads
.map {
meta, fastqs ->
return [meta, fastqs.flatten()]
}
.set { ch_fastq_lint }

//
// MODULE: Lint FastQ files
//
if (!skip_linting) {
FQ_LINT (
ch_fastq_lint
)
ch_versions = ch_versions.mix(FQ_LINT.out.versions.first())
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ch_reads
.map {
meta, fastqs ->
return [meta, fastqs.flatten()]
}
.set { ch_fastq_lint }
//
// MODULE: Lint FastQ files
//
if (!skip_linting) {
FQ_LINT (
ch_fastq_lint
)
ch_versions = ch_versions.mix(FQ_LINT.out.versions.first())
}
//
// MODULE: Lint FastQ files
//
if (!skip_linting) {
FQ_LINT (
ch_fastq_lint.map{ meta, fastqs -> [meta, fastqs.flatten()] }
)
ch_versions = ch_versions.mix(FQ_LINT.out.versions.first())
}

(but again, this needs PR'ing to the modules repo first)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the linting outputs should be added as a subworkflow output

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for reviewing @pinin4fjords ! I opened a PR in the modules repo here: nf-core/modules#7000

@pinin4fjords
Copy link
Member

Apologies, I ended up repeating this in #1461 after I'd improved the subworkflow (I had to do quite a bit of work on the testing suite).

But I'll be sure to credit you in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants