From c6d4950a603e2b4da0113789158068a541be1fd4 Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Wed, 1 Mar 2023 11:59:59 -0600 Subject: [PATCH 01/16] Add arity option for path inputs and outputs Signed-off-by: Ben Sherman --- docs/process.rst | 46 +++++++++++- .../nextflow/processor/TaskProcessor.groovy | 28 ++++--- .../nextflow/script/params/ArityParam.groovy | 74 +++++++++++++++++++ .../nextflow/script/params/FileInParam.groovy | 13 +++- .../script/params/FileOutParam.groovy | 13 +++- tests/process-arity.nf | 37 ++++++++++ 6 files changed, 197 insertions(+), 14 deletions(-) create mode 100644 modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy create mode 100644 tests/process-arity.nf diff --git a/docs/process.rst b/docs/process.rst index cd40d5d31b..4f12e2a24e 100644 --- a/docs/process.rst +++ b/docs/process.rst @@ -582,10 +582,10 @@ will output:: The target input file name may contain the ``*`` and ``?`` wildcards, which can be used to control the name of staged files. The following table shows how the wildcards are -replaced depending on the cardinality of the received input collection. +replaced depending on the arity of the received input collection. ============ ============== ================================================== -Cardinality Name pattern Staged file names +Arity Name pattern Staged file names ============ ============== ================================================== any ``*`` named as the source file 1 ``file*.ext`` ``file.ext`` @@ -650,6 +650,26 @@ with the current execution context. and some of these files may have the same filename. In this case, a solution would be to use the option ``stageAs``. + +Input file arity +---------------- + +.. note:: + This feature requires Nextflow version 23.04.0 or later. + +The *arity* of a ``path`` input is the number of files that it is expected to contain. The arity can be a +number or a range. Here are some examples:: + + input: + path('one.txt', arity: '1') // exactly one file is expected + path('pair_*.txt', arity: '2') // exactly two files are expected + path('many_*.txt', arity: '1..*') // one or more files are expected + path('optional.txt', arity: '0..1') // zero or one file is expected + +When a task is created, Nextflow will check whether the received files for each path input match the +declared arity, and fail if they do not. The default arity is ``'1..*'``. + + Input type ``env`` ------------------ @@ -1145,6 +1165,28 @@ on the actual value of the ``species`` input. because it will result in simpler and more portable code. +Output file arity +----------------- + +.. note:: + This feature requires Nextflow version 23.04.0 or later. + +The *arity* of a ``path`` output is the number of files that it is expected to contain. The arity can be a +number or a range. Here are some examples:: + + output: + path('one.txt', arity: '1') // exactly one file is expected + path('pair_*.txt', arity: '2') // exactly two files are expected + path('many_*.txt', arity: '1..*') // one or more files are expected + path('optional.txt', arity: '0..1') // zero or one file is expected (equivalent to optional: true) + +When a task completes, Nextflow will check whether the produced files for each path output match the +declared arity, and fail if they do not. + +The default arity is ``'1..*'``, or ``'0..*'`` if the output is declared optional. If you declare an +output as optional and also give it an arity, the optional flag will be ignored. + + .. _process-env: Output type ``env`` diff --git a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy index 5c44c6153d..3e2dd1ede7 100644 --- a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy @@ -1565,7 +1565,7 @@ class TaskProcessor { if( result ) allFiles.addAll(result) - else if( !param.optional ) { + else if( param.arity.min > 0 ) { def msg = "Missing output file(s) `$filePattern` expected by process `${safeTaskName(task)}`" if( inputsRemovedFlag ) msg += " (note: input files are not included in the default matching set)" @@ -1573,7 +1573,10 @@ class TaskProcessor { } } - task.setOutput( param, allFiles.size()==1 ? allFiles[0] : allFiles ) + if( !param.arity.contains(allFiles.size()) ) + throw new IllegalArgumentException("Incorrect number of output files for process `${safeTaskName(task)}` -- expected ${param.arity}, found ${allFiles.size()}") + + task.setOutput( param, param.arity.isSingle() ? allFiles[0] : allFiles ) } @@ -1816,10 +1819,10 @@ class TaskProcessor { return files } - protected singleItemOrList( List items, ScriptType type ) { + protected singleItemOrList( List items, boolean isSingle, ScriptType type ) { assert items != null - if( items.size() == 1 ) { + if( isSingle ) { return makePath(items[0],type) } @@ -2017,17 +2020,22 @@ class TaskProcessor { final param = entry.getKey() final val = entry.getValue() final fileParam = param as FileInParam - final normalized = normalizeInputToFiles(val, count, fileParam.isPathQualifier(), batch) - final resolved = expandWildcards( fileParam.getFilePattern(ctx), normalized ) - ctx.put( param.name, singleItemOrList(resolved, task.type) ) - count += resolved.size() - for( FileHolder item : resolved ) { + + def files = normalizeInputToFiles(val, count, fileParam.isPathQualifier(), batch) + files = expandWildcards( fileParam.getFilePattern(ctx), files ) + + if( !param.arity.contains(files.size()) ) + throw new IllegalArgumentException("Incorrect number of input files for process `${safeTaskName(task)}` -- expected ${param.arity}, found ${files.size()}") + + ctx.put( param.name, singleItemOrList(files, param.arity.isSingle(), task.type) ) + count += files.size() + for( FileHolder item : files ) { Integer num = allNames.getOrCreate(item.stageName, 0) +1 allNames.put(item.stageName,num) } // add the value to the task instance context - task.setInput(param, resolved) + task.setInput(param, files) } // -- set the delegate map as context in the task config diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy new file mode 100644 index 0000000000..9ca39380d7 --- /dev/null +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy @@ -0,0 +1,74 @@ +/* + * Copyright 2023, Seqera Labs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package nextflow.script.params + + +/** + * Implements an arity option for process inputs and outputs. + * + * @author Ben Sherman + */ +trait ArityParam { + + private Range arity = null + + def setArity(String arity) { + if( arity.isInteger() ) { + def n = arity.toInteger() + this.arity = new Range(n, n) + } + else if( arity.contains('..') && arity.tokenize('..').size() == 2 ) { + def (min, max) = arity.tokenize('..') + this.arity = new Range( + min == '*' ? 0 : min.toInteger(), + max == '*' ? Integer.MAX_VALUE : max.toInteger() + ) + } + else { + throw new IllegalArgumentException("Output path arity should be a number (e.g. '1') or range (e.g. '1..*')") + } + return this + } + + Range getArity() { + arity + } + + static class Range { + int min + int max + + Range(int min, int max) { + this.min = min + this.max = max + } + + boolean contains(int value) { + min <= value && value <= max + } + + boolean isSingle() { + min == 1 && max == 1 + } + + @Override + String toString() { + "${min}..${max == Integer.MAX_VALUE ? '*' : max}".toString() + } + } + +} diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy index 7148c5ea29..285092ef66 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy @@ -29,7 +29,7 @@ import nextflow.script.TokenVar */ @Slf4j @InheritConstructors -class FileInParam extends BaseInParam implements PathQualifier { +class FileInParam extends BaseInParam implements ArityParam, PathQualifier { protected filePattern @@ -121,6 +121,17 @@ class FileInParam extends BaseInParam implements PathQualifier { return value } + /** + * Override to initialize arity with default value. + */ + @Override + Range getArity() { + if( ArityParam.super.getArity() == null ) + ArityParam.super.setArity('1..*') + + ArityParam.super.getArity() + } + @Override FileInParam setPathQualifier(boolean flag) { pathQualifier = flag diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy index 50608f6886..3cf5607b69 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy @@ -34,7 +34,7 @@ import nextflow.util.BlankSeparatedList */ @Slf4j @InheritConstructors -class FileOutParam extends BaseOutParam implements OutParam, OptionalParam, PathQualifier { +class FileOutParam extends BaseOutParam implements OutParam, ArityParam, OptionalParam, PathQualifier { /** * ONLY FOR TESTING DO NOT USE @@ -262,6 +262,17 @@ class FileOutParam extends BaseOutParam implements OutParam, OptionalParam, Path return nameObj ? super.getName() : null } + /** + * Override to initialize arity with default value based on optional. + */ + @Override + Range getArity() { + if( ArityParam.super.getArity() == null ) + ArityParam.super.setArity( optional ? '0..*' : '1..*' ) + + ArityParam.super.getArity() + } + @Override FileOutParam setPathQualifier(boolean flag) { pathQualifier = flag diff --git a/tests/process-arity.nf b/tests/process-arity.nf new file mode 100644 index 0000000000..1fa121ce60 --- /dev/null +++ b/tests/process-arity.nf @@ -0,0 +1,37 @@ +#!/usr/bin/env nextflow + +process foo { + output: + path('one.txt', arity: '1') + path('pair_*.txt', arity: '2') + path('many_*.txt', arity: '1..*') + path('optional.txt', arity: '0..1') + script: + """ + echo 'one' > one.txt + echo 'pair_1' > pair_1.txt + echo 'pair_2' > pair_2.txt + echo 'many_1' > many_1.txt + echo 'many_2' > many_2.txt + echo 'many_3' > many_3.txt + """ +} + +process bar { + input: + path('one.txt', arity: '1') + path('pair_*.txt', arity: '2') + path('many_*.txt', arity: '1..*') + path('optional.txt', arity: '0..1') + script: + """ + cat one.txt + cat pair_*.txt + cat many_*.txt + [[ -f optional.txt ]] && cat optional.txt || true + """ +} + +workflow { + foo | bar +} \ No newline at end of file From 9a16f0108c1e0357e72ad6f66e7f55150e54e2f9 Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Thu, 2 Mar 2023 17:46:48 -0600 Subject: [PATCH 02/16] Change default arity to depend on file pattern Signed-off-by: Ben Sherman --- .../groovy/nextflow/processor/TaskProcessor.groovy | 6 +++--- .../groovy/nextflow/script/params/FileInParam.groovy | 6 +++++- .../groovy/nextflow/script/params/FileOutParam.groovy | 6 +++++- .../groovy/nextflow/processor/TaskProcessorTest.groovy | 10 ++++++++-- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy index 3e2dd1ede7..00e093e004 100644 --- a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy @@ -1576,7 +1576,7 @@ class TaskProcessor { if( !param.arity.contains(allFiles.size()) ) throw new IllegalArgumentException("Incorrect number of output files for process `${safeTaskName(task)}` -- expected ${param.arity}, found ${allFiles.size()}") - task.setOutput( param, param.arity.isSingle() ? allFiles[0] : allFiles ) + task.setOutput( param, allFiles.size()==1 && param.arity.isSingle() ? allFiles[0] : allFiles ) } @@ -1819,10 +1819,10 @@ class TaskProcessor { return files } - protected singleItemOrList( List items, boolean isSingle, ScriptType type ) { + protected singleItemOrList( List items, boolean single, ScriptType type ) { assert items != null - if( isSingle ) { + if( items.size() == 1 && single ) { return makePath(items[0],type) } diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy index 285092ef66..c2be4a9468 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy @@ -127,7 +127,11 @@ class FileInParam extends BaseInParam implements ArityParam, PathQualifier { @Override Range getArity() { if( ArityParam.super.getArity() == null ) - ArityParam.super.setArity('1..*') + ArityParam.super.setArity( + filePattern?.contains('*') || filePattern?.contains('?') + ? '1..*' + : '1' + ) ArityParam.super.getArity() } diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy index 3cf5607b69..0573bfb54f 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy @@ -268,7 +268,11 @@ class FileOutParam extends BaseOutParam implements OutParam, ArityParam, Optiona @Override Range getArity() { if( ArityParam.super.getArity() == null ) - ArityParam.super.setArity( optional ? '0..*' : '1..*' ) + ArityParam.super.setArity( + filePattern?.contains('*') || filePattern?.contains('?') + ? optional ? '0..*' : '1..*' + : optional ? '0..1' : '1' + ) ArityParam.super.getArity() } diff --git a/modules/nextflow/src/test/groovy/nextflow/processor/TaskProcessorTest.groovy b/modules/nextflow/src/test/groovy/nextflow/processor/TaskProcessorTest.groovy index 879e51f381..5386a871da 100644 --- a/modules/nextflow/src/test/groovy/nextflow/processor/TaskProcessorTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/processor/TaskProcessorTest.groovy @@ -186,13 +186,19 @@ class TaskProcessorTest extends Specification { when: def list = [ FileHolder.get(path1, 'x_file_1') ] - def result = processor.singleItemOrList(list, ScriptType.SCRIPTLET) + def result = processor.singleItemOrList(list, true, ScriptType.SCRIPTLET) then: result.toString() == 'x_file_1' + when: + list = [ FileHolder.get(path1, 'x_file_1') ] + result = processor.singleItemOrList(list, false, ScriptType.SCRIPTLET) + then: + result*.toString() == ['x_file_1'] + when: list = [ FileHolder.get(path1, 'x_file_1'), FileHolder.get(path2, 'x_file_2'), FileHolder.get(path3, 'x_file_3') ] - result = processor.singleItemOrList(list, ScriptType.SCRIPTLET) + result = processor.singleItemOrList(list, false, ScriptType.SCRIPTLET) then: result*.toString() == [ 'x_file_1', 'x_file_2', 'x_file_3'] From 4738a972f8989d7b048f8e61f5a63bf7b7d7310b Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Thu, 2 Mar 2023 17:58:44 -0600 Subject: [PATCH 03/16] Change arity "single"-ness to include optional single (`0..1`) Signed-off-by: Ben Sherman --- .../src/main/groovy/nextflow/script/params/ArityParam.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy index 9ca39380d7..6fdd8ea26e 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy @@ -62,7 +62,7 @@ trait ArityParam { } boolean isSingle() { - min == 1 && max == 1 + max == 1 } @Override From 058474f16214c54ad5b0c418e28b01f0045296f1 Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Wed, 8 Mar 2023 15:53:11 -0600 Subject: [PATCH 04/16] Remove default arity, use original behavior if arity not specified Signed-off-by: Ben Sherman --- .../nextflow/processor/TaskProcessor.groovy | 10 +++++----- .../nextflow/script/params/FileInParam.groovy | 15 --------------- .../nextflow/script/params/FileOutParam.groovy | 15 --------------- 3 files changed, 5 insertions(+), 35 deletions(-) diff --git a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy index 00e093e004..75c92cea8d 100644 --- a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy @@ -1565,7 +1565,7 @@ class TaskProcessor { if( result ) allFiles.addAll(result) - else if( param.arity.min > 0 ) { + else if( !param.optional && (!param.arity || param.arity.min > 0) ) { def msg = "Missing output file(s) `$filePattern` expected by process `${safeTaskName(task)}`" if( inputsRemovedFlag ) msg += " (note: input files are not included in the default matching set)" @@ -1573,10 +1573,10 @@ class TaskProcessor { } } - if( !param.arity.contains(allFiles.size()) ) + if( param.arity && !param.arity.contains(allFiles.size()) ) throw new IllegalArgumentException("Incorrect number of output files for process `${safeTaskName(task)}` -- expected ${param.arity}, found ${allFiles.size()}") - task.setOutput( param, allFiles.size()==1 && param.arity.isSingle() ? allFiles[0] : allFiles ) + task.setOutput( param, allFiles.size()==1 && (!param.arity || param.arity.isSingle()) ? allFiles[0] : allFiles ) } @@ -2024,10 +2024,10 @@ class TaskProcessor { def files = normalizeInputToFiles(val, count, fileParam.isPathQualifier(), batch) files = expandWildcards( fileParam.getFilePattern(ctx), files ) - if( !param.arity.contains(files.size()) ) + if( param.arity && !param.arity.contains(files.size()) ) throw new IllegalArgumentException("Incorrect number of input files for process `${safeTaskName(task)}` -- expected ${param.arity}, found ${files.size()}") - ctx.put( param.name, singleItemOrList(files, param.arity.isSingle(), task.type) ) + ctx.put( param.name, singleItemOrList(files, !param.arity || param.arity.isSingle(), task.type) ) count += files.size() for( FileHolder item : files ) { Integer num = allNames.getOrCreate(item.stageName, 0) +1 diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy index c2be4a9468..ed984ff12b 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy @@ -121,21 +121,6 @@ class FileInParam extends BaseInParam implements ArityParam, PathQualifier { return value } - /** - * Override to initialize arity with default value. - */ - @Override - Range getArity() { - if( ArityParam.super.getArity() == null ) - ArityParam.super.setArity( - filePattern?.contains('*') || filePattern?.contains('?') - ? '1..*' - : '1' - ) - - ArityParam.super.getArity() - } - @Override FileInParam setPathQualifier(boolean flag) { pathQualifier = flag diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy index 0573bfb54f..e6a46ea273 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy @@ -262,21 +262,6 @@ class FileOutParam extends BaseOutParam implements OutParam, ArityParam, Optiona return nameObj ? super.getName() : null } - /** - * Override to initialize arity with default value based on optional. - */ - @Override - Range getArity() { - if( ArityParam.super.getArity() == null ) - ArityParam.super.setArity( - filePattern?.contains('*') || filePattern?.contains('?') - ? optional ? '0..*' : '1..*' - : optional ? '0..1' : '1' - ) - - ArityParam.super.getArity() - } - @Override FileOutParam setPathQualifier(boolean flag) { pathQualifier = flag From 2a015c726c4a8baeb14576f927356b977dd59743 Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Wed, 8 Mar 2023 16:59:14 -0600 Subject: [PATCH 05/16] Add ArityParam unit test Signed-off-by: Ben Sherman --- .../nextflow/script/params/ArityParam.groovy | 39 +++++++++------- .../script/params/ArityParamTest.groovy | 44 +++++++++++++++++++ 2 files changed, 66 insertions(+), 17 deletions(-) create mode 100644 modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy index 6fdd8ea26e..a4a2e3df5b 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy @@ -16,36 +16,41 @@ package nextflow.script.params +import groovy.transform.CompileStatic /** * Implements an arity option for process inputs and outputs. * * @author Ben Sherman */ +@CompileStatic trait ArityParam { - private Range arity = null + Range arity - def setArity(String arity) { - if( arity.isInteger() ) { - def n = arity.toInteger() + Range getArity() { arity } + + def setArity(String value) { + if( value.isInteger() ) { + def n = value.toInteger() this.arity = new Range(n, n) + return this } - else if( arity.contains('..') && arity.tokenize('..').size() == 2 ) { - def (min, max) = arity.tokenize('..') - this.arity = new Range( - min == '*' ? 0 : min.toInteger(), - max == '*' ? Integer.MAX_VALUE : max.toInteger() - ) - } - else { - throw new IllegalArgumentException("Output path arity should be a number (e.g. '1') or range (e.g. '1..*')") + + def tokens = value.tokenize('..') + if( tokens.size() == 2 ) { + def min = tokens[0] + def max = tokens[1] + if( min.isInteger() && (max == '*' || max.isInteger()) ) { + this.arity = new Range( + min.toInteger(), + max == '*' ? Integer.MAX_VALUE : max.toInteger() + ) + return this + } } - return this - } - Range getArity() { - arity + throw new IllegalArgumentException("Path arity should be a number (e.g. '1') or a range (e.g. '1..*')") } static class Range { diff --git a/modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy b/modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy new file mode 100644 index 0000000000..284d548e44 --- /dev/null +++ b/modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy @@ -0,0 +1,44 @@ +/* + * Copyright 2023, Seqera Labs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package nextflow.script.params + +import spock.lang.Specification +import spock.lang.Unroll +/** + * + * @author Ben Sherman + */ +class ArityParamTest extends Specification { + + @Unroll + def 'should set arity' () { + + when: + def param = [:] as ArityParam + param.setArity(VALUE) + then: + param.arity.min == MIN + param.arity.max == MAX + + where: + VALUE | MIN | MAX + '1' | 1 | 1 + '0..1' | 0 | 1 + '1..*' | 1 | Integer.MAX_VALUE + } + +} From 4e17c9972b2d60ee316ab3820192fe40545784e7 Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Thu, 9 Mar 2023 09:48:29 -0600 Subject: [PATCH 06/16] Add tests for ArityParam (currently failing) Signed-off-by: Ben Sherman --- .../nextflow/script/params/ArityParam.groovy | 4 ++- .../script/params/ArityParamTest.groovy | 25 +++++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy index a4a2e3df5b..282cc6a888 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy @@ -72,7 +72,9 @@ trait ArityParam { @Override String toString() { - "${min}..${max == Integer.MAX_VALUE ? '*' : max}".toString() + min == max + ? min.toString() + : "${min}..${max == Integer.MAX_VALUE ? '*' : max}".toString() } } diff --git a/modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy b/modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy index 284d548e44..4104ba15f0 100644 --- a/modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy @@ -24,11 +24,15 @@ import spock.lang.Unroll */ class ArityParamTest extends Specification { + static class DefaultArityParam implements ArityParam { + DefaultArityParam() {} + } + @Unroll - def 'should set arity' () { + def testArity () { when: - def param = [:] as ArityParam + def param = new DefaultArityParam() param.setArity(VALUE) then: param.arity.min == MIN @@ -41,4 +45,21 @@ class ArityParamTest extends Specification { '1..*' | 1 | Integer.MAX_VALUE } + @Unroll + def testArityRange () { + + when: + def range = new ArityParam.Range(MIN, MAX) + then: + range.contains(2) == TWO + range.isSingle() == SINGLE + range.toString() == STRING + + where: + MIN | MAX | TWO | SINGLE | STRING + 1 | 1 | false | true | '1' + 0 | 1 | false | true | '0..1' + 1 | Integer.MAX_VALUE | true | false | '1..*' + } + } From 8c5e93cbec1eada1f6bc6e134aa8d2874380678a Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Thu, 30 Mar 2023 10:56:48 -0500 Subject: [PATCH 07/16] Add arity to params unit tests Signed-off-by: Ben Sherman --- .../groovy/nextflow/script/params/ArityParam.groovy | 5 +++++ .../groovy/nextflow/script/params/ParamsInTest.groovy | 11 +++++++---- .../nextflow/script/params/ParamsOutTest.groovy | 8 ++++++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy index 282cc6a888..3d81eb1979 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy @@ -70,6 +70,11 @@ trait ArityParam { max == 1 } + @Override + boolean equals(Object obj) { + min == obj.min && max == obj.max + } + @Override String toString() { min == max diff --git a/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsInTest.groovy b/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsInTest.groovy index e2ef5258ce..c96d223f16 100644 --- a/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsInTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsInTest.groovy @@ -706,12 +706,12 @@ class ParamsInTest extends Dsl2Spec { process hola { input: - path x - path f1 - path '*.fa' + path x, arity: '1' + path f1, arity: '1..2' + path '*.fa', arity: '1..*' path 'file.txt' path f2, name: '*.fa' - path f3, stageAs: '*.txt' + path f3, stageAs: '*.txt' return '' } @@ -737,18 +737,21 @@ class ParamsInTest extends Dsl2Spec { in0.inChannel.val == FILE in0.index == 0 in0.isPathQualifier() + in0.arity == new ArityParam.Range(1, 1) in1.name == 'f1' in1.filePattern == '*' in1.inChannel.val == FILE in1.index == 1 in1.isPathQualifier() + in1.arity == new ArityParam.Range(1, 2) in2.name == '*.fa' in2.filePattern == '*.fa' in2.inChannel.val == FILE in2.index == 2 in2.isPathQualifier() + in2.arity == new ArityParam.Range(1, Integer.MAX_VALUE) in3.name == 'file.txt' in3.filePattern == 'file.txt' diff --git a/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsOutTest.groovy b/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsOutTest.groovy index 1c46c898ba..d19820f4f6 100644 --- a/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsOutTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsOutTest.groovy @@ -931,7 +931,8 @@ class ParamsOutTest extends Dsl2Spec { separatorChar: '#', glob: false, optional: false, - includeInputs: false + includeInputs: false, + arity: '1' path y, maxDepth:5, @@ -941,7 +942,8 @@ class ParamsOutTest extends Dsl2Spec { separatorChar: ':', glob: true, optional: true, - includeInputs: true + includeInputs: true, + arity: '0..*' return '' } @@ -963,6 +965,7 @@ class ParamsOutTest extends Dsl2Spec { !out0.getGlob() !out0.getOptional() !out0.getIncludeInputs() + out0.getArity() == new ArityParam.Range(1, 1) and: out1.getMaxDepth() == 5 @@ -973,6 +976,7 @@ class ParamsOutTest extends Dsl2Spec { out1.getGlob() out1.getOptional() out1.getIncludeInputs() + out1.getArity() == new ArityParam.Range(0, Integer.MAX_VALUE) } def 'should set file options' () { From 04fbb86abd2af19122a8f05bc05a162cb82e210a Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Fri, 21 Apr 2023 11:24:15 -0500 Subject: [PATCH 08/16] Fix liftbot warning Signed-off-by: Ben Sherman --- .../main/groovy/nextflow/script/params/ArityParam.groovy | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy index 3d81eb1979..f259934854 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy @@ -17,6 +17,7 @@ package nextflow.script.params import groovy.transform.CompileStatic +import groovy.transform.EqualsAndHashCode /** * Implements an arity option for process inputs and outputs. @@ -53,6 +54,7 @@ trait ArityParam { throw new IllegalArgumentException("Path arity should be a number (e.g. '1') or a range (e.g. '1..*')") } + @EqualsAndHashCode static class Range { int min int max @@ -70,11 +72,6 @@ trait ArityParam { max == 1 } - @Override - boolean equals(Object obj) { - min == obj.min && max == obj.max - } - @Override String toString() { min == max From 7ec397fd262ce0186603cae6f7921966c9e44d55 Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Wed, 24 May 2023 17:53:40 +0200 Subject: [PATCH 09/16] Add support for AWS SSE env variables Signed-off-by: Paolo Di Tommaso --- .../nextflow/cloud/aws/config/AwsS3Config.groovy | 4 ++-- .../cloud/aws/config/AwsS3ConfigTest.groovy | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/plugins/nf-amazon/src/main/nextflow/cloud/aws/config/AwsS3Config.groovy b/plugins/nf-amazon/src/main/nextflow/cloud/aws/config/AwsS3Config.groovy index e479925314..5ff28aac75 100644 --- a/plugins/nf-amazon/src/main/nextflow/cloud/aws/config/AwsS3Config.groovy +++ b/plugins/nf-amazon/src/main/nextflow/cloud/aws/config/AwsS3Config.groovy @@ -50,8 +50,8 @@ class AwsS3Config { this.debug = opts.debug as Boolean this.endpoint = opts.endpoint ?: SysEnv.get('AWS_S3_ENDPOINT') this.storageClass = parseStorageClass((opts.storageClass ?: opts.uploadStorageClass) as String) // 'uploadStorageClass' is kept for legacy purposes - this.storageEncryption = parseStorageEncryption(opts.storageEncryption as String) - this.storageKmsKeyId = opts.storageKmsKeyId + this.storageEncryption = parseStorageEncryption(opts.storageEncryption as String) ?: SysEnv.get('NXF_AWS_SSE_MODE') + this.storageKmsKeyId = opts.storageKmsKeyId ?: SysEnv.get('NXF_AWS_SSE_KMS_KEY_ID') this.pathStyleAccess = opts.s3PathStyleAccess as Boolean this.s3Acl = parseS3Acl(opts.s3Acl as String) } diff --git a/plugins/nf-amazon/src/test/nextflow/cloud/aws/config/AwsS3ConfigTest.groovy b/plugins/nf-amazon/src/test/nextflow/cloud/aws/config/AwsS3ConfigTest.groovy index 116a64456b..0f5cd1f643 100644 --- a/plugins/nf-amazon/src/test/nextflow/cloud/aws/config/AwsS3ConfigTest.groovy +++ b/plugins/nf-amazon/src/test/nextflow/cloud/aws/config/AwsS3ConfigTest.groovy @@ -121,4 +121,18 @@ class AwsS3ConfigTest extends Specification { SysEnv.pop() } + + def 'should set storage encryption via env variable' () { + given: + SysEnv.push([NXF_AWS_SSE_MODE: 'aws:kms', NXF_AWS_SSE_KMS_KEY_ID: 'xyz1']) + + when: + def client = new AwsS3Config([:]) + then: + client.storageKmsKeyId == 'xyz1' + client.storageEncryption == 'aws:kms' + + cleanup: + SysEnv.pop() + } } From bbdca3ebb8d89a94e2c6cfa8a8fb9226af3a576e Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Tue, 15 Aug 2023 20:54:13 +0200 Subject: [PATCH 10/16] Update docs [ci skip] Signed-off-by: Paolo Di Tommaso --- docs/process.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/process.md b/docs/process.md index 9f4aec015c..bd4db434a4 100644 --- a/docs/process.md +++ b/docs/process.md @@ -474,7 +474,7 @@ workflow { Available options: `arity` -: :::{versionadded} 23.06.0-edge +: :::{versionadded} 23.09.0-edge ::: : Specify the number of expected files (default: `'1..*'`). Can be a number or a range: @@ -933,7 +933,7 @@ In the above example, the `randomNum` process creates a file named `result.txt` Available options: `arity` -: :::{versionadded} 23.06.0-edge +: :::{versionadded} 23.09.0-edge ::: : Specify the number of expected files (default: ``'1..*'``, or ``'0..*'`` if the output is declared optional). Can be a number or a range: From 88da8bb46a80969b6cfa41ca9903d9def2007840 Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Tue, 15 Aug 2023 18:15:37 -0500 Subject: [PATCH 11/16] Add nullable inputs/outputs and other improvements Signed-off-by: Ben Sherman --- docs/process.md | 16 ++++++---- .../nextflow/processor/TaskProcessor.groovy | 32 +++++++++++-------- .../nextflow/script/params/ArityParam.groovy | 22 ++++++++++--- .../script/params/ArityParamTest.groovy | 20 ++++++------ .../cloud/aws/config/AwsS3Config.groovy | 4 +-- .../cloud/aws/config/AwsS3ConfigTest.groovy | 14 -------- tests/process-arity.nf | 6 ++-- 7 files changed, 61 insertions(+), 53 deletions(-) diff --git a/docs/process.md b/docs/process.md index bd4db434a4..cfd6a3d317 100644 --- a/docs/process.md +++ b/docs/process.md @@ -476,7 +476,7 @@ Available options: `arity` : :::{versionadded} 23.09.0-edge ::: -: Specify the number of expected files (default: `'1..*'`). Can be a number or a range: +: Specify the number of expected files. Can be a number or a range: ```groovy input: @@ -488,6 +488,8 @@ Available options: When a task is created, Nextflow will check whether the received files for each path input match the declared arity, and fail if they do not. + An input is *nullable* if the arity is exactly `0..1`. Nullable inputs can accept "null" files from nullable `path` outputs. + `stageAs` : Specify how the file should be named in the task work directory: @@ -536,9 +538,9 @@ seq1 seq2 seq3 ... ``` -The target input file name may contain the `*` and `?` wildcards, which can be used to control the name of staged files. The following table shows how the wildcards are replaced depending on the arity of the received input collection. +The target input file name may contain the `*` and `?` wildcards, which can be used to control the name of staged files. The following table shows how the wildcards are replaced depending on the cardinality of the received input collection. -| Arity | Name pattern | Staged file names | +| Cardinality | Name pattern | Staged file names | | ----------- | ------------ | ------------------------------------------------------------------------------------------------------- | | any | `*` | named as the source file | | 1 | `file*.ext` | `file.ext` | @@ -935,19 +937,19 @@ Available options: `arity` : :::{versionadded} 23.09.0-edge ::: -: Specify the number of expected files (default: ``'1..*'``, or ``'0..*'`` if the output is declared optional). Can be a number or a range: +: Specify the number of expected files. Can be a number or a range: ```groovy output: path('one.txt', arity: '1') // exactly one file is expected path('pair_*.txt', arity: '2') // exactly two files are expected path('many_*.txt', arity: '1..*') // one or more files are expected - path('optional.txt', arity: '0..1') // zero or one file is expected (equivalent to optional: true) + path('optional.txt', arity: '0..1') // zero or one file is expected ``` - When a task completes, Nextflow will check whether the produced files for each path output match the declared arity, and fail if they do not. + When a task completes, Nextflow will check whether the produced files for each path output match the declared arity, and fail if they do not. If the arity is *single* (i.e. either `1` or `0..1`), a single file will be emitted. Otherwise, a list will always be emitted, even if only one file is produced. - If you declare an output as optional and also give it an arity, the optional flag will be ignored. + An output is *nullable* if the arity is exactly `0..1`. Whereas optional outputs emit nothing if the output file does not exist, nullable outputs emit a "null" file that can only be accepted by a nullable `path` input. `followLinks` : When `true` target files are return in place of any matching symlink (default: `true`) diff --git a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy index 12ef8d0cb0..f957607762 100644 --- a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy @@ -1567,6 +1567,10 @@ class TaskProcessor { def path = param.glob ? splitter.strip(filePattern) : filePattern def file = workDir.resolve(path) def exists = param.followLinks ? file.exists() : file.exists(LinkOption.NOFOLLOW_LINKS) + if( !exists && param.isNullable() ) { + file = null + exists = true + } if( exists ) result = [file] else @@ -1584,10 +1588,10 @@ class TaskProcessor { } } - if( param.arity && !param.arity.contains(allFiles.size()) ) + if( !param.isValidArity(allFiles.size()) ) throw new IllegalArgumentException("Incorrect number of output files for process `${safeTaskName(task)}` -- expected ${param.arity}, found ${allFiles.size()}") - task.setOutput( param, allFiles.size()==1 && (!param.arity || param.arity.isSingle()) ? allFiles[0] : allFiles ) + task.setOutput( param, allFiles.size()==1 && param.isSingle() ? allFiles[0] : allFiles ) } @@ -1789,7 +1793,7 @@ class TaskProcessor { if( obj instanceof Path ) return obj - if( !obj == null ) + if( obj == null ) throw new ProcessUnrecoverableException("Path value cannot be null") if( !(obj instanceof CharSequence) ) @@ -1808,7 +1812,7 @@ class TaskProcessor { throw new ProcessUnrecoverableException("Not a valid path value: '$str'") } - protected List normalizeInputToFiles( Object obj, int count, boolean coerceToPath, FilePorter.Batch batch ) { + protected List normalizeInputToFiles( Object obj, int count, boolean coerceToPath, boolean nullable, FilePorter.Batch batch ) { Collection allItems = obj instanceof Collection ? obj : [obj] def len = allItems.size() @@ -1816,6 +1820,8 @@ class TaskProcessor { // use a bag so that cache hash key is not affected by file entries order def files = new ArrayBag(len) for( def item : allItems ) { + if( item == null && nullable ) + continue if( item instanceof Path || coerceToPath ) { def path = normalizeToPath(item) @@ -2031,23 +2037,21 @@ class TaskProcessor { for( Map.Entry entry : secondPass.entrySet() ) { final param = entry.getKey() final val = entry.getValue() - final fileParam = param as FileInParam - - def files = normalizeInputToFiles(val, count, fileParam.isPathQualifier(), batch) - files = expandWildcards( fileParam.getFilePattern(ctx), files ) + final normalized = normalizeInputToFiles(val, count, param.isPathQualifier(), param.isNullable(), batch) + final resolved = expandWildcards( param.getFilePattern(ctx), normalized ) - if( param.arity && !param.arity.contains(files.size()) ) - throw new IllegalArgumentException("Incorrect number of input files for process `${safeTaskName(task)}` -- expected ${param.arity}, found ${files.size()}") + if( !param.isValidArity(resolved.size()) ) + throw new IllegalArgumentException("Incorrect number of input files for process `${safeTaskName(task)}` -- expected ${param.arity}, found ${resolved.size()}") - ctx.put( param.name, singleItemOrList(files, !param.arity || param.arity.isSingle(), task.type) ) - count += files.size() - for( FileHolder item : files ) { + ctx.put( param.name, singleItemOrList(resolved, param.isSingle(), task.type) ) + count += resolved.size() + for( FileHolder item : resolved ) { Integer num = allNames.getOrCreate(item.stageName, 0) +1 allNames.put(item.stageName,num) } // add the value to the task instance context - task.setInput(param, files) + task.setInput(param, resolved) } // -- set the delegate map as context in the task config diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy index f259934854..dc9045ee07 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy @@ -54,6 +54,24 @@ trait ArityParam { throw new IllegalArgumentException("Path arity should be a number (e.g. '1') or a range (e.g. '1..*')") } + /** + * Determine whether a null file is allowed. + */ + boolean isNullable() { + return arity && arity.min == 0 && arity.max == 1 + } + + /** + * Determine whether a single output file should be unwrapped. + */ + boolean isSingle() { + return !arity || arity.max == 1 + } + + boolean isValidArity(int size) { + return !arity || arity.contains(size) + } + @EqualsAndHashCode static class Range { int min @@ -68,10 +86,6 @@ trait ArityParam { min <= value && value <= max } - boolean isSingle() { - max == 1 - } - @Override String toString() { min == max diff --git a/modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy b/modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy index 4104ba15f0..70cbca64fe 100644 --- a/modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/script/params/ArityParamTest.groovy @@ -37,12 +37,15 @@ class ArityParamTest extends Specification { then: param.arity.min == MIN param.arity.max == MAX + param.isNullable() == NULLABLE + param.isSingle() == SINGLE where: - VALUE | MIN | MAX - '1' | 1 | 1 - '0..1' | 0 | 1 - '1..*' | 1 | Integer.MAX_VALUE + VALUE | NULLABLE | SINGLE | MIN | MAX + '1' | false | true | 1 | 1 + '0..1' | true | true | 0 | 1 + '1..*' | false | false | 1 | Integer.MAX_VALUE + '0..*' | false | false | 0 | Integer.MAX_VALUE } @Unroll @@ -52,14 +55,13 @@ class ArityParamTest extends Specification { def range = new ArityParam.Range(MIN, MAX) then: range.contains(2) == TWO - range.isSingle() == SINGLE range.toString() == STRING where: - MIN | MAX | TWO | SINGLE | STRING - 1 | 1 | false | true | '1' - 0 | 1 | false | true | '0..1' - 1 | Integer.MAX_VALUE | true | false | '1..*' + MIN | MAX | TWO | STRING + 1 | 1 | false | '1' + 0 | 1 | false | '0..1' + 1 | Integer.MAX_VALUE | true | '1..*' } } diff --git a/plugins/nf-amazon/src/main/nextflow/cloud/aws/config/AwsS3Config.groovy b/plugins/nf-amazon/src/main/nextflow/cloud/aws/config/AwsS3Config.groovy index 5ff28aac75..e479925314 100644 --- a/plugins/nf-amazon/src/main/nextflow/cloud/aws/config/AwsS3Config.groovy +++ b/plugins/nf-amazon/src/main/nextflow/cloud/aws/config/AwsS3Config.groovy @@ -50,8 +50,8 @@ class AwsS3Config { this.debug = opts.debug as Boolean this.endpoint = opts.endpoint ?: SysEnv.get('AWS_S3_ENDPOINT') this.storageClass = parseStorageClass((opts.storageClass ?: opts.uploadStorageClass) as String) // 'uploadStorageClass' is kept for legacy purposes - this.storageEncryption = parseStorageEncryption(opts.storageEncryption as String) ?: SysEnv.get('NXF_AWS_SSE_MODE') - this.storageKmsKeyId = opts.storageKmsKeyId ?: SysEnv.get('NXF_AWS_SSE_KMS_KEY_ID') + this.storageEncryption = parseStorageEncryption(opts.storageEncryption as String) + this.storageKmsKeyId = opts.storageKmsKeyId this.pathStyleAccess = opts.s3PathStyleAccess as Boolean this.s3Acl = parseS3Acl(opts.s3Acl as String) } diff --git a/plugins/nf-amazon/src/test/nextflow/cloud/aws/config/AwsS3ConfigTest.groovy b/plugins/nf-amazon/src/test/nextflow/cloud/aws/config/AwsS3ConfigTest.groovy index 0f5cd1f643..116a64456b 100644 --- a/plugins/nf-amazon/src/test/nextflow/cloud/aws/config/AwsS3ConfigTest.groovy +++ b/plugins/nf-amazon/src/test/nextflow/cloud/aws/config/AwsS3ConfigTest.groovy @@ -121,18 +121,4 @@ class AwsS3ConfigTest extends Specification { SysEnv.pop() } - - def 'should set storage encryption via env variable' () { - given: - SysEnv.push([NXF_AWS_SSE_MODE: 'aws:kms', NXF_AWS_SSE_KMS_KEY_ID: 'xyz1']) - - when: - def client = new AwsS3Config([:]) - then: - client.storageKmsKeyId == 'xyz1' - client.storageEncryption == 'aws:kms' - - cleanup: - SysEnv.pop() - } } diff --git a/tests/process-arity.nf b/tests/process-arity.nf index 1fa121ce60..4ecfba109c 100644 --- a/tests/process-arity.nf +++ b/tests/process-arity.nf @@ -5,7 +5,7 @@ process foo { path('one.txt', arity: '1') path('pair_*.txt', arity: '2') path('many_*.txt', arity: '1..*') - path('optional.txt', arity: '0..1') + path('nullable.txt', arity: '0..1') script: """ echo 'one' > one.txt @@ -22,13 +22,13 @@ process bar { path('one.txt', arity: '1') path('pair_*.txt', arity: '2') path('many_*.txt', arity: '1..*') - path('optional.txt', arity: '0..1') + path('nullable.txt', arity: '0..1') script: """ cat one.txt cat pair_*.txt cat many_*.txt - [[ -f optional.txt ]] && cat optional.txt || true + [[ -f nullable.txt ]] && cat nullable.txt || true """ } From 356a70d8905cad9feabcc644a28095be1a746031 Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Wed, 16 Aug 2023 10:03:54 -0500 Subject: [PATCH 12/16] Add e2e test Signed-off-by: Ben Sherman --- tests/checks/process-arity-fails.nf/.checks | 18 ++++++++++++++++++ tests/checks/process-arity.nf/.checks | 16 ++++++++++++++++ tests/process-arity-fails.nf | 19 +++++++++++++++++++ 3 files changed, 53 insertions(+) create mode 100644 tests/checks/process-arity-fails.nf/.checks create mode 100644 tests/checks/process-arity.nf/.checks create mode 100644 tests/process-arity-fails.nf diff --git a/tests/checks/process-arity-fails.nf/.checks b/tests/checks/process-arity-fails.nf/.checks new file mode 100644 index 0000000000..346e4a2ea0 --- /dev/null +++ b/tests/checks/process-arity-fails.nf/.checks @@ -0,0 +1,18 @@ +set +e + +# +# run normal mode +# +echo '' +$NXF_RUN +[[ $? == 0 ]] && exit 1 + + +# +# RESUME mode +# +echo '' +$NXF_RUN -resume +[[ $? == 0 ]] && exit 1 + +exit 0 \ No newline at end of file diff --git a/tests/checks/process-arity.nf/.checks b/tests/checks/process-arity.nf/.checks new file mode 100644 index 0000000000..66e9a3e265 --- /dev/null +++ b/tests/checks/process-arity.nf/.checks @@ -0,0 +1,16 @@ +set +e + +# +# run normal mode +# +echo '' +$NXF_RUN +[[ $? == 0 ]] || false + + +# +# RESUME mode +# +echo '' +$NXF_RUN -resume +[[ $? == 0 ]] || false \ No newline at end of file diff --git a/tests/process-arity-fails.nf b/tests/process-arity-fails.nf new file mode 100644 index 0000000000..9f2c62cc01 --- /dev/null +++ b/tests/process-arity-fails.nf @@ -0,0 +1,19 @@ +#!/usr/bin/env nextflow + +process foo { + output: + path('output.txt', arity: '0..1') + script: + true +} + +process bar { + input: + path(file) + script: + true +} + +workflow { + foo | bar +} \ No newline at end of file From b63ec869ae6b701211f17b36cabe4f55fa1b8ffb Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Wed, 16 Aug 2023 10:04:56 -0500 Subject: [PATCH 13/16] Add NullPath Signed-off-by: Ben Sherman --- .../nextflow/processor/TaskProcessor.groovy | 15 +++---- .../main/groovy/nextflow/util/NullPath.groovy | 41 +++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 modules/nextflow/src/main/groovy/nextflow/util/NullPath.groovy diff --git a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy index f957607762..68425680ee 100644 --- a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy @@ -103,6 +103,7 @@ import nextflow.util.CacheHelper import nextflow.util.Escape import nextflow.util.LockManager import nextflow.util.LoggerHelper +import nextflow.util.NullPath import nextflow.util.TestOnly import org.codehaus.groovy.control.CompilerConfiguration import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer @@ -1568,7 +1569,7 @@ class TaskProcessor { def file = workDir.resolve(path) def exists = param.followLinks ? file.exists() : file.exists(LinkOption.NOFOLLOW_LINKS) if( !exists && param.isNullable() ) { - file = null + file = new NullPath(path) exists = true } if( exists ) @@ -1789,11 +1790,14 @@ class TaskProcessor { return new FileHolder(source, result) } - protected Path normalizeToPath( obj ) { + protected Path normalizeToPath( obj, boolean nullable ) { + if( obj instanceof NullPath && !nullable ) + throw new ProcessUnrecoverableException("Path value cannot be null") + if( obj instanceof Path ) return obj - if( obj == null ) + if( !obj == null ) throw new ProcessUnrecoverableException("Path value cannot be null") if( !(obj instanceof CharSequence) ) @@ -1820,11 +1824,8 @@ class TaskProcessor { // use a bag so that cache hash key is not affected by file entries order def files = new ArrayBag(len) for( def item : allItems ) { - if( item == null && nullable ) - continue - if( item instanceof Path || coerceToPath ) { - def path = normalizeToPath(item) + def path = normalizeToPath(item, nullable) def target = executor.isForeignFile(path) ? batch.addToForeign(path) : path def holder = new FileHolder(target) files << holder diff --git a/modules/nextflow/src/main/groovy/nextflow/util/NullPath.groovy b/modules/nextflow/src/main/groovy/nextflow/util/NullPath.groovy new file mode 100644 index 0000000000..9ef1ab23c1 --- /dev/null +++ b/modules/nextflow/src/main/groovy/nextflow/util/NullPath.groovy @@ -0,0 +1,41 @@ +/* + * Copyright 2013-2023, Seqera Labs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package nextflow.util + +import java.nio.file.Path +import java.nio.file.Paths + +import groovy.transform.EqualsAndHashCode +import groovy.transform.PackageScope +import groovy.util.logging.Slf4j + +/** + * + * @author Ben Sherman + */ +@EqualsAndHashCode +@Slf4j +class NullPath implements Path { + + @PackageScope + @Delegate + Path delegate + + NullPath(String path) { + delegate = Paths.get(path) + } +} \ No newline at end of file From 76de19771f3e61880dfa122498710ef453b9b3b0 Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Wed, 16 Aug 2023 10:25:57 -0500 Subject: [PATCH 14/16] Fail if nullable input receives a list Signed-off-by: Ben Sherman --- .../nextflow/processor/TaskProcessor.groovy | 11 ++++++++--- .../nextflow/script/params/ArityParam.groovy | 17 +++++++++++++++-- tests/process-arity.nf | 4 ++-- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy index 68425680ee..3bc92aa5fd 100644 --- a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy @@ -1589,7 +1589,7 @@ class TaskProcessor { } } - if( !param.isValidArity(allFiles.size()) ) + if( !param.isValidArity(allFiles) ) throw new IllegalArgumentException("Incorrect number of output files for process `${safeTaskName(task)}` -- expected ${param.arity}, found ${allFiles.size()}") task.setOutput( param, allFiles.size()==1 && param.isSingle() ? allFiles[0] : allFiles ) @@ -1824,6 +1824,7 @@ class TaskProcessor { // use a bag so that cache hash key is not affected by file entries order def files = new ArrayBag(len) for( def item : allItems ) { + if( item instanceof Path || coerceToPath ) { def path = normalizeToPath(item, nullable) def target = executor.isForeignFile(path) ? batch.addToForeign(path) : path @@ -2041,8 +2042,12 @@ class TaskProcessor { final normalized = normalizeInputToFiles(val, count, param.isPathQualifier(), param.isNullable(), batch) final resolved = expandWildcards( param.getFilePattern(ctx), normalized ) - if( !param.isValidArity(resolved.size()) ) - throw new IllegalArgumentException("Incorrect number of input files for process `${safeTaskName(task)}` -- expected ${param.arity}, found ${resolved.size()}") + if( !param.isValidArity(resolved) ) { + final msg = param.isNullable() + ? "expected a nullable file (0..1) but a list was provided" + : "expected ${param.arity}, found ${resolved.size()}" + throw new IllegalArgumentException("Incorrect number of input files for process `${safeTaskName(task)}` -- ${msg}") + } ctx.put( param.name, singleItemOrList(resolved, param.isSingle(), task.type) ) count += resolved.size() diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy index dc9045ee07..0b61faebc4 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy @@ -68,8 +68,21 @@ trait ArityParam { return !arity || arity.max == 1 } - boolean isValidArity(int size) { - return !arity || arity.contains(size) + /** + * Determine whether a collection of files has valid arity. + * + * If the param is nullable, there should be exactly one file (either + * a real file or a null file) + * + * @param files + */ + boolean isValidArity(Collection files) { + if( !arity ) + return true + + return isNullable() + ? files.size() == 1 + : arity.contains(files.size()) } @EqualsAndHashCode diff --git a/tests/process-arity.nf b/tests/process-arity.nf index 4ecfba109c..9c2eca7030 100644 --- a/tests/process-arity.nf +++ b/tests/process-arity.nf @@ -22,13 +22,13 @@ process bar { path('one.txt', arity: '1') path('pair_*.txt', arity: '2') path('many_*.txt', arity: '1..*') - path('nullable.txt', arity: '0..1') + path(x, arity: '0..1') script: """ cat one.txt cat pair_*.txt cat many_*.txt - [[ -f nullable.txt ]] && cat nullable.txt || true + [[ -f ${x} ]] && cat ${x} || true """ } From 4be12369e3320b7c42e042fd003842c9ead54422 Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Wed, 16 Aug 2023 11:02:16 -0500 Subject: [PATCH 15/16] Add `arity: true` to infer arity from file pattern Signed-off-by: Ben Sherman --- docs/process.md | 6 +++++- .../groovy/nextflow/script/params/FileInParam.groovy | 10 ++++++++++ .../groovy/nextflow/script/params/FileOutParam.groovy | 10 ++++++++++ .../groovy/nextflow/script/params/ParamsInTest.groovy | 9 ++++++--- 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/docs/process.md b/docs/process.md index cfd6a3d317..592ce14064 100644 --- a/docs/process.md +++ b/docs/process.md @@ -490,6 +490,8 @@ Available options: An input is *nullable* if the arity is exactly `0..1`. Nullable inputs can accept "null" files from nullable `path` outputs. + You can also set `arity: true` to infer the arity from the file pattern: `1..*` if the pattern is a glob, `1` otherwise. + `stageAs` : Specify how the file should be named in the task work directory: @@ -508,7 +510,7 @@ Available options: } ``` - Cab be a name or a pattern as described in the [Multiple input files](#multiple-input-files) section. + Can be a name or a pattern as described in the [Multiple input files](#multiple-input-files) section. ### Multiple input files @@ -951,6 +953,8 @@ Available options: An output is *nullable* if the arity is exactly `0..1`. Whereas optional outputs emit nothing if the output file does not exist, nullable outputs emit a "null" file that can only be accepted by a nullable `path` input. + You can also set `arity: true` to infer the arity from the file pattern: `1..*` if the pattern is a glob, `1` otherwise. + `followLinks` : When `true` target files are return in place of any matching symlink (default: `true`) diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy index 8bebf83d70..ae0ac4da5f 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/FileInParam.groovy @@ -153,4 +153,14 @@ class FileInParam extends BaseInParam implements ArityParam, PathQualifier { return this } + def setArity(boolean value) { + if( !value ) + return + + final str = filePattern?.contains('*') || filePattern?.contains('?') + ? '1..*' + : '1' + setArity(str) + } + } diff --git a/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy b/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy index a55cd957c7..085e662449 100644 --- a/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/script/params/FileOutParam.groovy @@ -276,4 +276,14 @@ class FileOutParam extends BaseOutParam implements OutParam, ArityParam, Optiona (FileOutParam)super.setOptions(opts) } + def setArity(boolean value) { + if( !value ) + return + + final str = filePattern?.contains('*') || filePattern?.contains('?') + ? '1..*' + : '1' + setArity(str) + } + } diff --git a/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsInTest.groovy b/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsInTest.groovy index 3312a37a3c..8f8a033a92 100644 --- a/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsInTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/script/params/ParamsInTest.groovy @@ -710,9 +710,9 @@ class ParamsInTest extends Dsl2Spec { path x, arity: '1' path f1, arity: '1..2' path '*.fa', arity: '1..*' - path 'file.txt' - path f2, name: '*.fa' - path f3, stageAs: '*.txt' + path 'file.txt', arity: true + path f2, name: '*.fa', arity: true + path f3, stageAs: '*.txt', arity: true return '' } @@ -759,16 +759,19 @@ class ParamsInTest extends Dsl2Spec { in3.inChannel.val == FILE in3.index == 3 in3.isPathQualifier() + in3.arity == new ArityParam.Range(1, 1) in4.name == 'f2' in4.filePattern == '*.fa' in4.index == 4 in4.isPathQualifier() + in4.arity == new ArityParam.Range(1, Integer.MAX_VALUE) in5.name == 'f3' in5.filePattern == '*.txt' in5.index == 5 in5.isPathQualifier() + in5.arity == new ArityParam.Range(1, Integer.MAX_VALUE) } def 'test input paths with gstring'() { From b6a5fb37066baf722a0d38a1461b03830338c1fe Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Wed, 16 Aug 2023 11:10:37 -0500 Subject: [PATCH 16/16] Fix failing tests Signed-off-by: Ben Sherman --- .../src/main/groovy/nextflow/processor/TaskProcessor.groovy | 2 +- .../src/test/groovy/nextflow/processor/TaskProcessorTest.groovy | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy index 3bc92aa5fd..8c468898e4 100644 --- a/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy @@ -1790,7 +1790,7 @@ class TaskProcessor { return new FileHolder(source, result) } - protected Path normalizeToPath( obj, boolean nullable ) { + protected Path normalizeToPath( obj, boolean nullable=false ) { if( obj instanceof NullPath && !nullable ) throw new ProcessUnrecoverableException("Path value cannot be null") diff --git a/modules/nextflow/src/test/groovy/nextflow/processor/TaskProcessorTest.groovy b/modules/nextflow/src/test/groovy/nextflow/processor/TaskProcessorTest.groovy index 471699f812..71bafeb50d 100644 --- a/modules/nextflow/src/test/groovy/nextflow/processor/TaskProcessorTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/processor/TaskProcessorTest.groovy @@ -829,7 +829,7 @@ class TaskProcessorTest extends Specification { def proc = new TaskProcessor(); proc.executor = executor when: - def result = proc.normalizeInputToFiles(PATH.toString(), 0, true, batch) + def result = proc.normalizeInputToFiles(PATH.toString(), 0, true, false, batch) then: 1 * executor.isForeignFile(PATH) >> false 0 * batch.addToForeign(PATH) >> null