Add arity option for path inputs and outputs#3706
Add arity option for path inputs and outputs#3706bentsherman wants to merge 23 commits intomasterfrom
Conversation
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy
Show resolved
Hide resolved
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
This should be avoided. it could create nasty side effects hard depending the nextflow version. Arity should default to |
|
Should be able to make that work. It's too bad though, because the current defaults would probably work 99% of the time without the user having to specify. It would be nice if we could eventually transition to these default settings instead of just |
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
|
ArityParam fails on my laptop but passes in CI 🤷 |
0d59b4c to
b93634e
Compare
|
Worth adding some tests for the DSL both for input and output, like in this example |
|
I think that is covered by the e2e test I added: tests/process-arity.nf |
|
Better to add also as unit test |
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
modules/nextflow/src/main/groovy/nextflow/script/params/ArityParam.groovy
Show resolved
Hide resolved
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
|
I found a glitch testing this. If I'm not wrong, when declaring an input `arity: '0..1', it could be a null or a path type, however, I've noticed that when an empty list is given the input type is a list. This was my test case |
|
I think I wasn't sure how to handle that case. You think the list should be coerced into a single null path? |
|
Now I'm thinking we shouldn't allow the arity to include 0, and instead only support that through the nullable option #2893 |
|
Should be null according this
Instead I believe we should drop |
That could work, actually I think that would be much simpler
Well, I was only thinking about outputs when I wrote this. If an input has |
|
Also I think I will add the ability to specify |
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
|
I added the The problem with the null path is that it goes through a bunch of code that it probably shouldn't, and produces this warning: |
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
|
I'm thinking about the zero one infinity rule and wondering if we should only allow the following:
It seems to be that other cases like // instead of arity: '2'
tuple path('1.txt'), path('2.txt')
// instead of arity: '1..2'
tuple path('1.txt'), path('2.txt', arity: '0..1') |
|
I don't think such a restriction would simplify this PR much, and there might be some weird cases where something like |
81f7cb7 to
8a43489
Compare
|
I've merged this manually, removing the support for |
|
Thanks. I will rebase this branch and make a new PR with just the optional features |
Closes #969 #1236 #2425 #3341
This PR adds an
arityoption for path inputs and outputs. The arity can be a single number (e.g.1) or a range (e.g.1..*). The arity is used (1) to validate the expected number of files and (2) to determine, when a path contains only one file, whether to "wrap" that path in a list.Currently, a path with a single file is always unwrapped. With this PR, the path will be unwrapped only if there is a single file and the arity is "single", meaning it can have at most 1 element (in other words,
1and0..1are "single").The default arity is
1, or1..*if the param has a glob pattern. For optional output params, the default then is0..1or0..*respectively.This PR introduces a minor breaking change -- a path input/output with a glob pattern will have a default arity of
1..*, so it will be wrapped in a list even if there is only one file. Without this PR, this file would be unwrapped. I think this breaking change is worthwhile since it is exactly the inconsistency that users were complaining about.