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

modules config update - preprocessing refactor #485

Merged
merged 33 commits into from
Feb 20, 2022

Conversation

maxulysse
Copy link
Member

@maxulysse maxulysse commented Feb 15, 2022

  • Update all modules
  • Add back params.publish_dir_mode
  • refactor --skip_qc, --skip_markduplicates and --skip_bqsr into --skip_tools
  • move out split_fastq into its own subworkflow
  • move out prepare_intervals into its own subworkflow
  • Use ext.when for modules in all current subworkflows
    • annotate.nf
    • bam2fastq.nf
    • germline_variant_calling.nf
    • mapping_csv.nf
    • markduplicates_csv.nf
    • pair_copy_number_calling.nf
    • pair_variant_calling.nf
    • prepare_genome.nf
    • prepare_intervals.nf
    • prepare_recalibration_csv.nf
    • recalibrate_csv.nf
    • split_fastq.nf
    • tumor_variant_calling.nf
    • fastqc_trimgalore.nf
    • markduplicates.nf
    • prepare_recalibration.nf
    • recalibrate.nf
    • annotation_ensemblvep/main.nf
    • annotation_snpeff/main.nf
    • fgbio_create_umi_consensus/main.nf
    • gatk4_mapping/main.nf
    • gatk_tumor_normal_somatic_variant_calling/main.nf
    • gatk_tumor_only_somatic_variant_calling/main.nf
    • joint_germline_variant_calling/main.nf

conf/modules.config Outdated Show resolved Hide resolved
@@ -89,7 +89,7 @@ chr_dir = params.chr_dir ? Channel.fromPath(params.chr_dir).
chr_length = params.chr_length ? Channel.fromPath(params.chr_length).collect() : []
dbsnp = params.dbsnp ? Channel.fromPath(params.dbsnp).collect() : Channel.empty()
fasta = params.fasta ? Channel.fromPath(params.fasta).collect() : []
germline_resource = params.germline_resource ? Channel.fromPath(params.germline_resource).collect() : []
germline_resource = params.germline_resource ? Channel.fromPath(params.germline_resource).collect() : Channel.empty()
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the difference again using [] and Channel.empty() ?

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure, but it wasn't working

Copy link
Member

Choose a reason for hiding this comment

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

When creating a process input set, using Channel.empty() won't trigger the process to run, but if you give it [], the process runs with [] as the value for the corresponding parameter.
That's why channel.ifEmpty([]) is used so much for MULTIQC.

nextflow.config Outdated Show resolved Hide resolved
nextflow.config Outdated Show resolved Hide resolved
@maxulysse maxulysse marked this pull request as ready for review February 18, 2022 14:44
@maxulysse maxulysse changed the title modules config update modules config update - preprocessing refactor Feb 18, 2022
@maxulysse
Copy link
Member Author

Stopping the refactoring here not to have a PR so huge it's a black hole that will swallow all the code reviewers

@FriederikeHanssen FriederikeHanssen self-requested a review February 18, 2022 14:58
Copy link
Contributor

@FriederikeHanssen FriederikeHanssen left a comment

Choose a reason for hiding this comment

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

looks good to me, not sure why the CI tests are not passing. only the variant_calling one should fail.

conf/modules.config Outdated Show resolved Hide resolved
@maxulysse
Copy link
Member Author

looks good to me, not sure why the CI tests are not passing. only the variant_calling one should fail.

OK, I'll figure out the rest of the CI and then call it a day

umi {
params.input = "${baseDir}/tests/csv/3.0/fastq_umi.csv"
params.umi_read_structure = '7M1S+T'
}
use_gatk_spark {
params.use_gatk_spark = 'baserecalibrator,markduplicates'
}
}

//This is apparently useless as it won't overwrite things in the modules.config
process {
Copy link
Contributor

Choose a reason for hiding this comment

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

Think we can delete this now, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why not, we still want to test this no?

Copy link
Contributor

Choose a reason for hiding this comment

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

it doesn't get overwritten but will need to be moved to the modules.config and activated somehow on test

@maxulysse
Copy link
Member Author

maxulysse commented Feb 20, 2022

Only gatkspark test falling (on top on the previously variant calling too), but can't manage to make it work on my computer, and figure the current dev is actually not running gatk spark...
So I'll merge as it is

@maxulysse maxulysse merged commit 66d66e0 into nf-core:dev Feb 20, 2022
@maxulysse maxulysse deleted the dev_skip branch February 20, 2022 12:26
@github-actions
Copy link

Markdown linting is failing

To keep the code consistent with lots of contributors, we run automated code consistency checks.
To fix this CI test, please run:

  • Install markdownlint-cli
  • Fix the markdown errors
    • Automatically: markdownlint . --fix
    • Manually resolve anything left from markdownlint .

Once you push these changes the test should pass, and you can hide this comment 👍

We highly recommend setting up markdownlint in your code editor so that this formatting is done automatically on save. Ask about it on Slack for help!

Thanks again for your contribution!

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.

3 participants