Skip to content

fix issue #5612 - boolean param to optionally log skipped fastqs - #190

Closed
glichtenstein wants to merge 24 commits into
nf-core:devfrom
glichtenstein:boolean-parameter-to--log_skipped_fastqs
Closed

fix issue #5612 - boolean param to optionally log skipped fastqs#190
glichtenstein wants to merge 24 commits into
nf-core:devfrom
glichtenstein:boolean-parameter-to--log_skipped_fastqs

Conversation

@glichtenstein

@glichtenstein glichtenstein commented May 21, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes nf-core/modules#5612
nf-core/modules PR counterpart: nf-core/modules#5638

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/demultiplex branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core 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).

Description

This pull request adds the optional parameter --log_skipped_fastqs to the demultiplex pipeline. This change was necessary to address the issue with logging skipped FASTQ files and preventing the AWS S3 issue.

Patch Details

The changes in this PR were applied using a patch from a specific commit in the nf-core/modules repository. The commit details are as follows:

The patch was applied to ensure that the demultiplex repository includes the latest changes from nf-core/modules necessary for this feature.

Testing

true
nextflow run glichtenstein/demultiplex -r 7d9538e --input nf-core-samplesheet.csv --outdir /data/scratch/iseq-DI/output -profile docker -work-dir /data/scratch/iseq-DI/workdir -resume --skip_tools fastp --log_empty_fastqs=true
false
nextflow run glichtenstein/demultiplex -r 7d9538e --input nf-core-samplesheet.csv --outdir /data/scratch/iseq-DI/output -profile docker -work-dir /data/scratch/iseq-DI/workdir -resume --skip_tools fastp --log_empty_fastqs=false

executor > local (5)
[1a/e1ae22] process > NFCORE_DEMULTIPLEX:DEMULTIPLEX:BCL_DEMULTIPLEX:BCLCONVERT [100%] 1 of 1 ✔
[5f/05036f] process > NFCORE_DEMULTIPLEX:DEMULTIPLEX:FALCO (iseq-DI_S1_L001) [100%] 1 of 1 ✔
[84/64923f] process > NFCORE_DEMULTIPLEX:DEMULTIPLEX:MD5SUM (iseq-DI_S1_L001) [100%] 2 of 2 ✔
[11/351698] process > NFCORE_DEMULTIPLEX:DEMULTIPLEX:MULTIQC [100%] 1 of 1 ✔
-[nf-core/demultiplex] Pipeline completed successfully-

cat output/empty_fastqs.log
Empty or invalid FASTQ file: /data/scratch/iseq-DI/workdir/1a/e1ae22.../Bad-iseq-DI_S2_L001_R1_001.fastq.gz
Empty or invalid FASTQ file: /data/scratch/iseq-DI/workdir/1a/e1ae22.../Bad-iseq-DI_S2_L001_R2_001.fastq.gz

Dataset used

  • A public BCL from 10X Genomics. I will add it to nf-core tests datasets when possible.
mkdir -p iseq-DI; cd iseq-DI
wget https://cf.10xgenomics.com/supp/spatial-exp/demultiplexing/iseq-DI.tar.gz
wget https://cf.10xgenomics.com/supp/spatial-exp/demultiplexing/bcl_convert_samplesheet.csv

SampleSheet was modified to include bad barcodes.

nf-core_samplesheet.csv

id,samplesheet,lane,flowcell,per_flowcell_manifest
AA0BCDFGH,/data/scratch/iseq-DI/bcl_convert_samplesheet.csv,1,/data/scratch/iseq-DI/iseq-DI.tar.gz

bcl_convert_samplesheet.csv

[Header],,,
FileFormatVersion,2,,
,,,
[BCLConvert_Settings],,,
CreateFastqForIndexReads,0,,
,,,
[BCLConvert_Data],,,
Lane,Sample_ID,index,index2
1,iseq-DI,GTAACATGCG,AGGTAACACT
1,Bad-iseq-DI,TATACAGATA,GATACAGATA

@glichtenstein
glichtenstein requested review from Aratz and k1sauce May 21, 2024 05:54
@glichtenstein
glichtenstein requested review from a team and blajoie as code owners May 21, 2024 05:54
@github-actions

This comment was marked as resolved.

Sign in to view
@github-actions

github-actions Bot commented May 21, 2024 •
edited
Loading

Copy link
Copy Markdown

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit c12458d

+| ✅ 182 tests passed       |+
#| ❔   3 tests were ignored |#
!| ❗   6 tests had warnings |!
Details

❗ Test warnings:

  • pipeline_todos - TODO string in README.md: Describe the minimum required steps to execute the pipeline, e.g. how to prepare samplesheets.
  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your preferred methods description, e.g. add publication citation for this pipeline
  • pipeline_todos - TODO string in awsfulltest.yml: You can customise AWS full pipeline tests as required

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 2.14.1
  • Run at 2024-05-31 21:27:21

@glichtenstein glichtenstein mentioned this pull request May 21, 2024
Closed
2 tasks
@glichtenstein
glichtenstein changed the base branch from master to dev May 21, 2024 14:08
@glichtenstein glichtenstein changed the title Boolean parameter to --log_skipped_fastqs fix issue #5612 - boolean param to optionally log skipped fastqs May 21, 2024
@glichtenstein

glichtenstein commented May 21, 2024

Copy link
Copy Markdown
Contributor Author

It will need the the nf-core/modules PR counterpart to pass the tests.
nf-core/modules#5638

@glichtenstein glichtenstein self-assigned this May 21, 2024
@glichtenstein glichtenstein added the Ready to review PR ready for review label May 21, 2024
@glichtenstein glichtenstein mentioned this pull request May 21, 2024
Closed
17 tasks
@glichtenstein
glichtenstein force-pushed the boolean-parameter-to--log_skipped_fastqs branch from 910cba5 to 3806b6a Compare May 23, 2024 23:16
@glichtenstein
glichtenstein force-pushed the boolean-parameter-to--log_skipped_fastqs branch from 4fc95ef to fffb21d Compare May 23, 2024 23:44
@glichtenstein
glichtenstein force-pushed the boolean-parameter-to--log_skipped_fastqs branch from 0d6ca1d to 6531e13 Compare May 24, 2024 16:31
@glichtenstein
glichtenstein force-pushed the boolean-parameter-to--log_skipped_fastqs branch 2 times, most recently from 34478cd to 3a89595 Compare May 31, 2024 17:58
@glichtenstein

glichtenstein commented Jun 1, 2024

Copy link
Copy Markdown
Contributor Author

Will close it in favor of nf-core/modules#5734 with help of @k1sauce refactoring of demultiplex.nf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Aratz Aratz Awaiting requested review from Aratz
@k1sauce k1sauce Awaiting requested review from k1sauce
@blajoie blajoie Awaiting requested review from blajoie

Assignees

@glichtenstein glichtenstein

Labels

Ready to review PR ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

new File breaks cloud execution in bcl_demultiplex

1 participant

@glichtenstein