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

95 remove hardcoded nextflowconfig paths #97

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

azmigueldario
Copy link

  • Remove hardcoded paths to databases in nextflow.config
  • Correct evaluation of database files
  • Remove analysis of --point mutations in resfinder if a species is not specified
  • Add paths to databases specific to the Eagle cluster in a separate config

PR checklist

  • [ X ] This comment contains a description of changes (with reason).
  • [ X ] 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/bacpaq branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • [ X ] Ensure the test suite passes (nf-test test main.nf.test -profile test,docker).
  • 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).

@azmigueldario azmigueldario linked an issue Mar 12, 2025 that may be closed by this pull request
nextflow.config Outdated
save_trimmed_fail = false // Check if reads failed in the trimming process need to be saved (True/False, default: False)
save_merged = true // Check if merged reads need to be saved (True/False, default: True)
trim_tool = "fastp" // Select which trimming tool to use (fastp/trimmomatic/trimgalore)
adapter_fasta = null // location for adapator database (fasta file)
Copy link
Member

Choose a reason for hiding this comment

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

"null" expected by fastp

Copy link
Author

Choose a reason for hiding this comment

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

Corrected, changed to 'null'

nanopore_summary_file = "/mnt/cidgoh-object-storage/hackathon/seqqc/isolate_wgs/nanopore/run_summary/sequencing_summary_FAT24492_18678559.txt"
// ID for nanopore summary file to be used by PycoQC
nanopore_summary_file_id = 'test'
nanopore_summary_file = null // Path to nanopore summary file to be used by PycoQC
Copy link
Member

Choose a reason for hiding this comment

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

This is correct, but need to update the nanopore_raw_reads_qc subworkflow to check for file. If file is not provided, empty channel [[],[]] should be passed

Copy link
Author

@azmigueldario azmigueldario Mar 18, 2025

Choose a reason for hiding this comment

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

Added logic evaluation to run PYCOQC only if summary file is available. Name is obtained directly from summary file instead of the nanopore_summary_file_id

Test: working with and without providing the path to the summary file

nextflow.config Outdated
// ID for nanopore summary file to be used by PycoQC
nanopore_summary_file_id = 'test'
nanopore_summary_file = null // Path to nanopore summary file to be used by PycoQC
nanopore_summary_file_id = 'test' // ID for nanopore summary file to be used by PycoQC
Copy link
Member

Choose a reason for hiding this comment

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

remove this parameter and update in the subworkflow to take file name as meta.id

Copy link
Author

Choose a reason for hiding this comment

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

Fix: params.nanopore_summary_file is used to obtain an id tag

classified_reads = true
// Boolean for whether to output unclassified reads as a fastq file
unclassified_reads = true
kraken2_db = null // Path to Kraken2 database used for querying reads during taxonomic classification
Copy link
Member

Choose a reason for hiding this comment

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

when specifying null for a path value, we need to test if it throws an error when the module/sub-workflow is skipped

Copy link
Author

Choose a reason for hiding this comment

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

fix: improved logical evaluation of databases in taxonomy_qc, updated centrifuge pipeline

// Validate database paths
//

if (!params.skip_kraken2) {
Copy link
Member

Choose a reason for hiding this comment

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

move these tests to specific subworkflows

Copy link
Author

@azmigueldario azmigueldario Mar 18, 2025

Choose a reason for hiding this comment

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

Fix: moved to taxonomy_qc, raw_reads_qc, or assembly_qc

Copy link
Member

@anwarMZ anwarMZ left a comment

Choose a reason for hiding this comment

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

test for the database paths when null. move the validation to specific sub-workflows.

@azmigueldario azmigueldario requested a review from anwarMZ March 18, 2025 20:28
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.

Remove hardcoded nextflow.config paths
2 participants