nf-core smrnaseq release 1.0 #9

chuan-wang · 2018-08-08T11:47:16Z

Refactors to nf-core smrnaseq;
Option for adding sequencing center to bam files from aligner;
Switch to Bowtie 1 for the alignment to host reference genome.

First nf-core version

apeltzer

Minor comments by me.

apeltzer · 2018-08-09T08:13:11Z

conf/uppmax-modules.config

+  enabled = false
+}
+
+process {


This still has the old process$ syntax you replaced further up in the other configs - maybe that would be good here too?

apeltzer · 2018-08-09T08:13:37Z

conf/uppmax-modules.config

+
+process {
+  // Environment modules and resource requirements
+  $makeBowtieIndex.module = ['bioinfo-tools', 'Fastx/0.0.14', 'bowtie/1.2.0']


Same here - process$ instead of withName:

apeltzer · 2018-08-09T08:16:37Z

conf/uppmax.config

@@ -13,6 +13,7 @@
 process {
  executor = 'slurm'
  clusterOptions = { "-A $params.project ${params.clusterOptions ?: ''}" }
+  container = "docker://$wf_container"


You don#t need the docker prefix anymore.
And you could have something like in the base profile here: https://github.com/nf-core/rnaseq/blob/2cf2f0f779d39463b09d874449cbb9fdb9f58d2f/conf/base.config#L14
inside the profile here too - initializing the variable in nextflow.config https://github.com/nf-core/rnaseq/blob/2cf2f0f779d39463b09d874449cbb9fdb9f58d2f/nextflow.config#L15

…efix

chuan-wang · 2018-08-09T12:13:43Z

This PR closes #10

ewels

Looks good! A few comments, though all minor things.

ewels · 2018-08-09T12:19:20Z

main.nf

@@ -84,6 +85,31 @@ if (params.help){
    exit 0
 }

+// Pipeline options
+params.name = false


Please don't move all of these into main.nf - they should be in nextflow.config. This is because the nf-core linting tests we do use the nextflow config -flat . to pull out all of the config options. This nextflow command doesn't look in main.nf sadly.

ewels · 2018-08-09T12:20:31Z

nextflow.config

  nf_required_version = '0.30.2' // Minimum version of nextflow required
  container = 'nfcore/smrnaseq:latest' // Container slug. Stable releases should specify release tag!!
-
-  // Pipeline options
-  params.name = false


I guess you moved these to main.nf because they weren't working here. The reason is because of this code is inside the params { block starting on L11 but also prefixes the variables with params.. So this means that the variables end up being for example params.params.name.

To fix, just remove the params. prefix on these lines 👍

ewels · 2018-08-09T12:22:27Z

README.md

-
-**nf-core/smrnaseq** is a bioinformatics best-practice analysis pipeline used for small RNA sequencing data at the [National Genomics Infastructure](https://ngisweden.scilifelab.se/)
-at [SciLifeLab Stockholm](https://www.scilifelab.se/platforms/ngi/), Sweden.
+**nf-core/smrnaseq** is a bioinformatics best-practice analysis pipeline used for small RNA sequencing data. It is developed at the [National Genomics Infastructure](https://ngisweden.scilifelab.se/) at [SciLifeLab Stockholm](https://www.scilifelab.se/platforms/ngi/), Sweden.


Please remove the stuff that's specifically references SciLifeLab like this. See https://github.com/nf-core/methylseq for an example of this introduction text.

ewels · 2018-08-09T12:23:04Z

README.md


-<p align="center"><a href="stand_alone/http://www.scilifelab.se/" target="_blank"><img src="docs/images/SciLifeLab_logo.png" title="SciLifeLab"></a></p>
+### Credits
+These scripts were originally written for use at the [National Genomics Infrastructure](https://portal.scilifelab.se/genomics/) at [SciLifeLab](http://www.scilifelab.se/) in Stockholm, Sweden, by Phil Ewels (@ewels), Chuan Wang (@chuan-wang) and Rickard Hammarén (@Hammarn)


This bit is fine :)

ewels · 2018-08-09T12:38:40Z

Singularity

+    VERSION 1.0
+
+%environment
+    PATH=/opt/conda/envs/nfcore-smrnaseq-1.0/bin:$PATH


The below %post command also needs updating sorry (I forgot this when we were talking before). Should be:

%post /opt/conda/bin/conda env create -f /environment.yml /opt/conda/bin/conda clean -a

ewels · 2018-08-09T12:44:02Z

main.nf

@@ -313,6 +340,7 @@ process bowtie_miRBase_mature {
    script:
    index_base = index.toString().tokenize(' ')[0].tokenize('.')[0]
    prefix = reads.toString() - ~/(.R1)?(_R1)?(_trimmed)?(\.fq)?(\.fastq)?(\.gz)?$/
+    seqCenter = params.seqCenter ? "--sam-RG ID:${prefix} --sam-RG 'CN:${params.seqCenter}'" : ''


To remove all whitespace from the string, you can use .${params.seqCenter.replaceAll("\\s","")}:

seqCenter = params.seqCenter ? "--sam-RG ID:${prefix} --sam-RG 'CN:${params.seqCenter.replaceAll("\\s","")}'" : ''

ewels · 2018-08-09T12:44:45Z

main.nf

@@ -325,6 +353,7 @@ process bowtie_miRBase_mature {
        --strata \\
        -e 99999 \\
        --chunkmbs 2048 \\
+        $seqCenter \\


Minor thing, but for these kinds of flags I usually don't put them on their own line. It should work fine, but most people won't set it and will have a blank line in their command which can look kind of scary in the logs.

ewels · 2018-08-09T12:44:54Z

main.nf

@@ -351,6 +380,7 @@ process bowtie_miRBase_hairpin {
    script:
    index_base = index.toString().tokenize(' ')[0].tokenize('.')[0]
    prefix = reads.toString() - '.mature_unmapped.fq.gz'
+    seqCenter = params.seqCenter ? "--sam-RG ID:${prefix} --sam-RG 'CN:${params.seqCenter}'" : ''


As above with removing whitespace

ewels · 2018-08-09T12:45:08Z

main.nf

@@ -363,6 +393,7 @@ process bowtie_miRBase_hairpin {
        -e 99999 \\
        --chunkmbs 2048 \\
        -q <(zcat $reads) \\
+        $seqCenter \\


As above about putting it on the same line as another option

ewels · 2018-08-09T13:05:53Z

main.nf

+            --strata \\
+            -e 99999 \\
+            --chunkmbs 2048 \\
+            $seqCenter \\


As above with the newlines

ewels · 2018-08-10T08:37:28Z

Final thing @chuan-wang - please could you update the conda tools to their latest version? This is something we should do for each release:

trim-galore=0.4.5, 0.5.0 available
samtools=1.8, 1.9 available
multiqc=1.5, 1.6 available

(please mention in the changelog)

apeltzer

Hmm, why are you moving these to the main.nf and not keeping these in nextflow.config as usual?

chuan-wang · 2018-08-10T09:45:04Z

Hi, the pipeline only ran with the parameters of reference in main.nf.

apeltzer · 2018-08-10T09:47:41Z

Hmm, I'll have a look later. Will land in a couple of minutes and loose internet connection therefore...

ewels

A couple of final changes! Sorry! Nearly there....

ewels · 2018-08-10T11:46:44Z

CHANGELOG.md

+* Add option for sequencing centre in BAM file.
+* Software updates: trim-galore 0.4.5 to 0.5.0; samtools 1.8 to 1.9; multiqc 1.5 to 1.6.
+
+Change bowtie parameters from `bowtie -n x -l 15 -k 10 --best (x=0 for mature and x=1 for hairpin)` into `bowtie -k 1 -m 1 --best --strata`.

 ## [0.1dev](https://github.com/SciLifeLab/NGI-smRNAseq/releases/tag/0.1dev) - 2018-05-14


Please remove this section - instead just have a line saying that the pipeline was originally called https://github.com/SciLifeLab/NGI-smRNAseq and that previous versions can be seen there.

ewels · 2018-08-10T11:47:55Z

Dockerfile

@@ -4,4 +4,4 @@ LABEL authors="alex.peltzer@gmail.com" \
    description="Docker image containing all requirements for nf-core/smrnaseq pipeline"

 COPY environment.yml /
-RUN conda env update -n root -f /environment.yml && conda clean -a 


As you updated the Singularity file to use the new (reverted) syntax where we do create instead of update -n root, please also update the Dockerfile here.

ewels · 2018-08-10T12:15:01Z

nextflow.config

-  params.skip_fastqc = false
-  params.skip_multiqc = false
+  name = false
+  project = false


Looks like singleEnd has gone missing here.

Correction: we don't ever run paired end data in this pipeline, so it doesn't make sense to define params.singleEnd in this pipeline. So the warning from the lint tests is acceptable.

…version

drpatelh · 2019-04-26T21:54:58Z

@ewels @apeltzer @chuan-wang

How far off is this PR to being merged? We will need to do some major updates to this pipeline in order to bring it in line with the latest template code. I don't see a dev branch either which is why I'm wondering how we should manage this PR.

apeltzer · 2019-04-26T22:58:44Z

Maybe we could change this to merge in a new branch dev and work on it afterwards ?

_{Sent with GitHawk}

drpatelh · 2019-04-26T23:01:18Z

Yes. That makes sense. Ill try and update the template code when I get a moment.

drpatelh · 2019-04-27T16:37:47Z

Ive added a dev branch and changed the base branch for this PR.

Ill leave it up to you guys as to how you want to merge/fix this PR because you have requested changes. Also, lint and pipeline tests were failing at the time with Travis CI...

lpantano · 2019-05-21T18:16:02Z

Hi all,

I would really like to work on the pipeline, anything I can do to push this forward and start implementing some other tools and strategies?

Can I address the revisions?

Thanks!

drpatelh · 2019-05-21T18:24:59Z

That would be great @lpantano ! Sorry, I haven't had time to update the template code.

I think we should maybe just merge this PR as it stands and then you can develop the pipeline from that point on to get everything working.

lpantano · 2019-05-21T18:26:39Z

I am happy with that, maybe that works better, so +1 from me.

apeltzer · 2019-05-21T18:41:02Z

Go ahead - glad if someone takes care @lpantano !

apeltzer · 2019-05-21T18:41:57Z

Ok I just merged this to dev - please create a branch off it and then improve the open issues / points on it to make this work / follow the latest template :-)

lpantano · 2019-05-21T18:42:45Z

perfect! thanks.

Template update for nf-core/tools version 1.9

chuan-wang added 9 commits August 6, 2018 11:07

Merge pull request #6 from nf-core/master

5d3b79e

First nf-core version

Large refactor for nf-core

267b9cd

Change retry strategy for some QC steps

fe9c38f

Add uppmax-modules config

1a339a2

Small refactors

5001922

Small refactors

c6e6465

Correct parameter conflict issue

9c06d5c

Fix bugs

e7b5c6f

Replace old deprecated process syntax with withName in base.config

a74c393

chuan-wang requested a review from ewels August 8, 2018 12:17

This was referenced Aug 8, 2018

Use bowtie 1 for human genome alignment #4

Closed

Add option for sequencing centre in BAM file #6

Closed

apeltzer requested changes Aug 9, 2018

View reviewed changes

chuan-wang added 4 commits August 9, 2018 10:29

Replace dollar signs with withName; in uppmax.config remove docker pr…

c2e9a6a

…efix

Fix conda env issue when running on UPPMAX

6a0741d

Merge branch 'master' into master

6501033

Fix all documents

fdc4eab

chuan-wang mentioned this pull request Aug 9, 2018

Add usage documentation #10

Closed

ewels requested changes Aug 9, 2018

View reviewed changes

ewels added the awaiting-changes label Aug 9, 2018

Small refactors based on Phil's comments

0554b18

ewels removed the awaiting-changes label Aug 9, 2018

chuan-wang added 2 commits August 10, 2018 10:57

Update versions of trim-galore, samtools and multiqc

997e627

Move reference params from nextflow.config into main.nf

0450098

apeltzer reviewed Aug 10, 2018

View reviewed changes

ewels requested changes Aug 10, 2018

View reviewed changes

chuan-wang added 3 commits August 10, 2018 14:27

Update Dockerfile and CHANGELOG

1b07bf8

Add protocol with pre-defined settings; Add miRTrace; Update MultiQC …

ef70b0c

…version

More specifications about miRTrace output files

9d68afd

chuan-wang force-pushed the master branch from 268682a to 9d68afd Compare January 16, 2019 13:11

drpatelh changed the base branch from master to dev April 27, 2019 16:33

apeltzer merged commit f83cb2a into nf-core:dev May 21, 2019

nschcolnicov pushed a commit that referenced this pull request Oct 10, 2024

Merge pull request #9 from drpatelh/TEMPLATE

b6ae2a3

Template update for nf-core/tools version 1.9

nf-core smrnaseq release 1.0 #9

nf-core smrnaseq release 1.0 #9

Conversation

chuan-wang commented Aug 8, 2018

apeltzer 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

chuan-wang commented Aug 9, 2018

ewels 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

ewels commented Aug 10, 2018

apeltzer left a comment

Choose a reason for hiding this comment

chuan-wang commented Aug 10, 2018

apeltzer commented Aug 10, 2018

ewels 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

drpatelh commented Apr 26, 2019

apeltzer commented Apr 26, 2019

drpatelh commented Apr 26, 2019

drpatelh commented Apr 27, 2019

lpantano commented May 21, 2019

drpatelh commented May 21, 2019

lpantano commented May 21, 2019

apeltzer commented May 21, 2019

apeltzer commented May 21, 2019

lpantano commented May 21, 2019