Dsl2 refactor #338

maxulysse · 2021-01-28T10:41:33Z

PR checklist

modules/nf-core/software/bwamem2_index.nf

…te path to modules/subworflows

abhi18av · 2021-02-08T18:30:08Z

What's the plan to add DeepVariant to the DSL2 version?

This current PR looks pretty extensive, any plans on merging this one to dsl2 branch soon? I can use that one (after the merge) as a foundation.

maxulysse · 2021-02-08T20:00:02Z

Hi @abhi18av
You got the main idea.
I do need a couple of more stuff to be fully up to date, but it should arrive soon.
Current plan is to merge this PR to dsl2 by the end of the week, that way we will have caught up with the current DSL2 best practices.
And from there, start to work on deepvariant and other issues.

abhi18av · 2021-02-08T20:02:18Z

Ah, cool 👍

Also, I wanted to ask whether you'd like to include the stub feature as well while we are at it? This should make the dev iterations faster.

maxulysse · 2021-02-08T20:10:10Z

That's a point I need to check with the @nf-core/core team.
But I do like the idea

maxulysse · 2021-02-11T12:25:21Z

OK, so I think current state is good enough for me for now.
and from that we can really start working on finishing the port.
There's still some linting issue to figure out, but that'll be done when tools is updated and ready for DSL2

maxulysse · 2021-02-11T12:29:59Z

There will also be more modules update, but I really think that we can merge that before, and then go crazy with the modules and (sub)workflows.
I think there's room for improvement in the current sarek workflow (in the workflow folder).
I'll try to put more code in lib/ so that we can have dedicated workflow depending on what we want to do.
-> thinking about regular sarek, sarek with minimal genome, sarek with sentieon, etc...

modules/nf-core/software/bwa/mem/functions.nf

…T_VCF and MULTIQC

FriederikeHanssen

So beautiful 😍 I just had some minor comments. One thing, I am wondering though is, whether it would make sense to tag the conda versions more specifically as you did in the latest environment.yml: bwa-mem2=2.0=he513fc3_1 and so on

FriederikeHanssen · 2021-02-12T09:11:04Z

.github/workflows/ci.yml

@@ -2,8 +2,6 @@ name: nf-core CI
 # This workflow runs the pipeline with the minimal test dataset to check that it completes without any syntax errors
 on:
  push:
-    branches:


is this now running on all branches? or none?

FriederikeHanssen · 2021-02-12T09:12:33Z

.github/workflows/ci.yml

@@ -21,6 +19,7 @@ jobs:
      matrix:
        # Nextflow versions: check pipeline minimum
        nxf_ver: ['20.11.0-edge']
+        engine: ['docker']


any reason to not add singularity and conda? To keep the tests short? I am just thinking that given we pull from 3 different sources now, this could catch incorrect urls and such quickly

random spaces accidentally added somewhere in between has caused me a few issues here and there 😄

Plan is to do that, but not implemented yet

FriederikeHanssen · 2021-02-12T09:15:04Z

README.md

@@ -22,12 +25,28 @@ Sarek can also handle tumour / normal pairs and could include additional relapse

 The pipeline is built using [`Nextflow`](https://www.nextflow.io), a workflow tool to run tasks across multiple compute infrastructures in a very portable manner. It comes with `Docker` containers making installation trivial and results highly reproducible.


It comes with 'Docker'... is this still up-to date or should we add the other container engines, since we don't have a monolithic Docker file anymore?

Good catch, we might need to update the TEMPLATE

-> nf-core/tools#850

that was fast 🚒

FriederikeHanssen · 2021-02-12T09:16:38Z

README.md

+
+* Sequencing quality control (`FastQC`)
+* Map Reads to Reference (`BWA mem`)
+* Mark Duplicates (`GATK MarkDuplicatesSpark`)


Should be MarkDuplicates due to param changes

Suggested change

* Mark Duplicates (`GATK MarkDuplicatesSpark`)

* Mark Duplicates (`GATK MarkDuplicates`)

FriederikeHanssen · 2021-02-12T09:23:01Z

README.md

+
+If you would like to contribute to this pipeline, please see the [contributing guidelines](.github/CONTRIBUTING.md).
+
+For further information or help, don't hesitate to get in touch on the [Slack `#sarek` channel](https://nfcore.slack.com/channels/sarek) (you can join with [this invite](https://nf-co.re/join/slack)), or contact us: [Maxime Garcia](mailto:maxime.garcia@scilifelab.se?subject=[GitHub]%20nf-core/sarek), [Szilvester Juhos](mailto:szilveszter.juhos@scilifelab.se?subject=[GitHub]%20nf-core/sarek)


Suggested change

For further information or help, don't hesitate to get in touch on the [Slack `#sarek` channel](https://nfcore.slack.com/channels/sarek) (you can join with [this invite](https://nf-co.re/join/slack)), or contact us: [Maxime Garcia](mailto:maxime.garcia@scilifelab.se?subject=[GitHub]%20nf-core/sarek), [Szilvester Juhos](mailto:szilveszter.juhos@scilifelab.se?subject=[GitHub]%20nf-core/sarek)

For further information or help, don't hesitate to get in touch on the [Slack `#sarek` channel](https://nfcore.slack.com/channels/sarek) (you can join with [this invite](https://nf-co.re/join/slack)), or contact us: [Maxime Garcia](mailto:maxime.garcia@scilifelab.se?subject=[GitHub]%20nf-core/sarek), [Szilvester Juhos](mailto:szilveszter.juhos@scilifelab.se?subject=[GitHub]%20nf-core/sarek), [Friederike Hanssen](mailto:friederike.hanssen@qbic.uni-tuebingen.de?subject=[GitHub]%20nf-core/sarek), [Gisela Gabernet](mailto:gisela.gabernet@qbic.uni-tuebingen.de?subject=[GitHub]%20nf-core/sarek)

FriederikeHanssen · 2021-02-12T09:26:08Z

modules/local/concat_vcf.nf

@@ -15,8 +12,12 @@ process CONCAT_VCF {
    publishDir params.outdir, mode: params.publish_dir_mode,
        saveAs: { filename -> saveFiles(filename:filename, options:params.options, publish_dir:getSoftwareName(task.process), publish_id:meta.id) }

-    conda environment
-    container container
+    conda (params.enable_conda ? "bioconda::htslib=1.11" : null)


Does this also have a build number? --hd3b49d5_0 I mean?

Good point, I'll look into it

FriederikeHanssen · 2021-02-12T09:27:34Z

modules/nf-core/software/bwa/mem/main.nf

+        mode: params.publish_dir_mode,
+        saveAs: { filename -> saveFiles(filename:filename, options:params.options, publish_dir:getSoftwareName(task.process), publish_id:meta.id) }
+
+    conda (params.enable_conda ? "bioconda::bwa=0.7.17 bioconda::samtools=1.10" : null)


Why not use samtoll 1.11? as in the other steps? Ah I guess, it is from the modules repo and not a local one, right?

Because I did not check if there was a mulled container for it actually...
I just used the modules from nf-core/modules when there was one.
Was not planning to update tools in this PR.
I'm saving that for the future PRs

FriederikeHanssen · 2021-02-12T09:30:08Z

modules/nf-core/software/functions.nf

-        if (ioptions.publish_by_id) path_list.add(args.publish_id)
+        if (ioptions.publish_by_id) {
+            path_list.add(args.publish_id)
+        }


Since the functions file is now within each tool/module folder, is this one still needed?

maxulysse added 30 commits January 28, 2021 11:09

chores: update CHANGELOG

a6203ed

chores: update bwa-mem2 + samtools

8ae205c

chores: update README

deb6fcb

chores: remove obsolete script

d5eccfd

chores: remove obsolete files

aa60a0e

chores: enable CI tests on all branches

8654fa5

chores: Remove readcube link

7c5c975

chores: update docs about containers

1a3c194

chores: print help refactor + comments header

41e8889

chores: update groovy libs

b3695bb

chores: improved CI

6cc20f1

chores: update groovy lib

eb50c10

chores: remove comments and unused code

b4225b8

chores: remove obsolete github actions

84a297b

fix: consistent URLs for doi resolver + actually use the doi for sarek

c9e35ab

chores: actually use the get_genome_attribute function

2739e4e

chores: refactor conda, docker and singularity declaration

5ce928f

fix: conda, docker, singularity declaration

a848c69

fix: better management of gatk_spark container

308dfc9

chores: use nf-core modules for bwa/index

53c14a3

chores: use nf-core/modules for fastqc

6bce561

chores: use nf-core/modules multiqc

9cb6cdc

chores: use nf-core/modules samtools/index and samtools/stats

a219405

chores: use nf-core/modules trimgalore

c9ba13e

fix: default should be GRCh38 not GRCh37

6e0971b

fix: correct container for bwa/index

d2d36dc

fix: actually a quay.io container

0fc5a05

chores: use nf-core/modules bwa/mem

bc7bd84

chores: update file

bf838da

chores: formatting

131e948

maxulysse added 3 commits February 2, 2021 20:30

fix: CI tests from input data repository

21e3745

chores: update bwamem2_mem to port to nf-core/modules

0b5ab05

fix: remove unused functions copied from nf-core/rnaseq

fbb9afc

FriederikeHanssen reviewed Feb 2, 2021

View reviewed changes

modules/nf-core/software/bwamem2_index.nf Outdated Show resolved Hide resolved

maxulysse added 4 commits February 3, 2021 08:05

chores: update bwa-mem2 to 2.1

dd38bae

chores: move subworkflows to a separate folder

c8bb74a

chores: move local modules, update fastqc_trimgalor subworkflow, upda…

44cdf21

…te path to modules/subworflows

fix: bwa-mem2

3f66150

maxulysse added 2 commits February 10, 2021 08:49

chores: update CHANGELOG

4c47582

chores: new Sarek workflow file

ae2d6b8

maxulysse marked this pull request as ready for review February 11, 2021 12:23

maxulysse requested review from FriederikeHanssen and a team February 11, 2021 12:23

FriederikeHanssen reviewed Feb 11, 2021

View reviewed changes

modules/nf-core/software/bwa/mem/functions.nf Show resolved Hide resolved

maxulysse added 2 commits February 11, 2021 17:56

fix: remove warning for no process matching config selector BWAMEM2_MEM

b8c379c

fix: remove warning for no process matching config selector for CONCA…

f4b8b43

…T_VCF and MULTIQC

FriederikeHanssen approved these changes Feb 12, 2021

View reviewed changes

maxulysse merged commit a51856e into nf-core:dsl2 Feb 12, 2021

maxulysse deleted the dsl2_refactor branch February 12, 2021 09:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dsl2 refactor #338

Dsl2 refactor #338

maxulysse commented Jan 28, 2021 •

edited

Loading

abhi18av commented Feb 8, 2021

maxulysse commented Feb 8, 2021

abhi18av commented Feb 8, 2021 •

edited

Loading

maxulysse commented Feb 8, 2021

maxulysse commented Feb 11, 2021

maxulysse commented Feb 11, 2021

FriederikeHanssen left a comment

FriederikeHanssen Feb 12, 2021

FriederikeHanssen Feb 12, 2021

FriederikeHanssen Feb 12, 2021

maxulysse Feb 12, 2021

FriederikeHanssen Feb 12, 2021

maxulysse Feb 12, 2021

maxulysse Feb 12, 2021

FriederikeHanssen Feb 12, 2021

FriederikeHanssen Feb 12, 2021

FriederikeHanssen Feb 12, 2021

FriederikeHanssen Feb 12, 2021

maxulysse Feb 12, 2021

FriederikeHanssen Feb 12, 2021

maxulysse Feb 12, 2021

FriederikeHanssen Feb 12, 2021

		@@ -22,12 +25,28 @@ Sarek can also handle tumour / normal pairs and could include additional relapse

		The pipeline is built using [`Nextflow`](https://www.nextflow.io), a workflow tool to run tasks across multiple compute infrastructures in a very portable manner. It comes with `Docker` containers making installation trivial and results highly reproducible.

	* Mark Duplicates (`GATK MarkDuplicatesSpark`)
	* Mark Duplicates (`GATK MarkDuplicates`)


		If you would like to contribute to this pipeline, please see the [contributing guidelines](.github/CONTRIBUTING.md).

		For further information or help, don't hesitate to get in touch on the [Slack `#sarek` channel](https://nfcore.slack.com/channels/sarek) (you can join with [this invite](https://nf-co.re/join/slack)), or contact us: [Maxime Garcia](mailto:maxime.garcia@scilifelab.se?subject=[GitHub]%20nf-core/sarek), [Szilvester Juhos](mailto:szilveszter.juhos@scilifelab.se?subject=[GitHub]%20nf-core/sarek)

Dsl2 refactor #338

Dsl2 refactor #338

Conversation

maxulysse commented Jan 28, 2021 • edited Loading

PR checklist

abhi18av commented Feb 8, 2021

maxulysse commented Feb 8, 2021

abhi18av commented Feb 8, 2021 • edited Loading

maxulysse commented Feb 8, 2021

maxulysse commented Feb 11, 2021

maxulysse commented Feb 11, 2021

FriederikeHanssen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxulysse commented Jan 28, 2021 •

edited

Loading

abhi18av commented Feb 8, 2021 •

edited

Loading