nextflow-io / patterns

A curated collection of Nextflow implementation patterns
http://nextflow-io.github.io/patterns/
MIT License
332 stars 71 forks source link

Update workflows to DSL2 #26

Closed bentsherman closed 2 years ago

bentsherman commented 2 years ago

Resolves #23

This PR updates all workflow patterns to DSL2.

Notes:

TODO:

pditommaso commented 2 years ago

This is great! however I've tried a couple and there are not working. e.g

process foo {
  errorStrategy 'ignore'
  script:
  '''
  echo This is going to fail!
  exit 1
  '''
}

process bar {
  script:
  '''
  echo OK
  '''
}

workflow {
  foo & bar
}

and

process foo {
  publishDir 'results', saveAs: { filename -> filename.endsWith(".zip") ? "zips/$filename" : filename }

  output:
  path '*'

  '''
  touch this.txt
  touch that.zip
  '''
}

workflow {
  foo
}

Broken docs is much worse than not having docs. Those should be tested one by one before merging.

bentsherman commented 2 years ago

Okay it looks like I was misunderstanding the DSL2 syntax here. I assumed that

workflow {
  foo
}

would simply call foo, but it looks like you have to explicitly invoke foo(), and foo by itself is only valid when using pipes. I will review and correct my changes accordingly.

bentsherman commented 2 years ago

The reason I didn't catch these problems is because the syntax is still valid, the workflow doesn't fail but simply runs nothing and exits. Might warrant some clarification in the Nextflow docs.

For example, consider these examples:

// runs foo and then bar
workflow {
  foo | bar
}

// does nothing
workflow {
  foo
}

// does nothing
workflow {
  foo & bar
}
bentsherman commented 2 years ago

Okay I made the changes you requested 👍

pditommaso commented 2 years ago

This is wrong

process foo {
  echo true
  input:
  file x

  script:
  """
  echo your_command --input $x
  """
}

workflow {
  foo("$baseDir/data/reads/*_1.fq.gz")
}

because the input is declared as file and the concrete input is a string therefore the process will convert the path string into a file :/

think it's a good opportunity to change all file to path in the input and output definitions

bentsherman commented 2 years ago

You got it. Only exception is process-per-file-chunk.nf, because it uses behavior of file to convert implicitly text into file chunks. If you're trying to get away from that pattern, you can use the split command instead like I do here.

One other question is, do you want to replace echo with debug ? It will remove the deprecation warning but then it won't work with Nextflow < 22.04.

pditommaso commented 2 years ago

You got it. Only exception is process-per-file-chunk.nf, because it uses behavior of file to convert implicitly text into file chunks

Ok, that's fine to keep it as example why to use file instead of path 👍

One other question is, do you want to replace echo with debug

Yes, let's go with debug!

bentsherman commented 2 years ago

Ready to go 👍

pditommaso commented 2 years ago

¡Vamos!