Skip to content

Conversation

bentsherman
Copy link
Member

Implements default values for process inputs as requested in #3507

While we could merge this feature as is, I'd like to try to implement optional and named inputs as well, to make sure that they are designed to work together properly.

@pditommaso
Copy link
Member

This is a good start. If I understand it correctly it works at the level of the process composition ie if the argument is provided matches the ones expected.

But I think there are use cases in which the argument is provided, but the associated channel brings no value (or null).

A classic example may be this

@bentsherman
Copy link
Member Author

I think the case of a null value could be handled with a separate nullable option as discussed here.

If the channel is empty, then the user can use ifEmpty() to provide a default value. We could have Nextflow do this automatically if a default value is defined in the process, but I don't know if that is always desirable. It might cause a process to be executed when the user didn't want it. I have to think about this one more.

@bentsherman bentsherman linked an issue Mar 3, 2023 that may be closed by this pull request
@fabianegli
Copy link

In #3714 it is noted that

The main caveat is that inputs with default values should be declared after inputs with no default, otherwise it's useless.

Is that really a necessary constraint to have? And what does "useless" actually mean on a technical level? It is not quite clear to me from reading through this and the other issue.

@bentsherman
Copy link
Member Author

@fabianegli see the docs example I added in this PR for a more detailed explanation. If your process declares an input B with no default value, after an input A with a default value, then you will always have to give a value for A when you call the process, because B comes after A. Thus the default value for A is useless because it can never be used.

I think the caveat effectively goes away if combined with named inputs, because then you can specify the inputs in any order when you call the process.

@fabianegli
Copy link

I understand the restriction an I am wondering what purpose it might serve. And my point is that I can't think of any situation I'd actually want it. If the defaults are introduced with named inputs, which removes this restriction I see this as something which might surprise some pipeline/module devs rather than a problem.

@bentsherman
Copy link
Member Author

And my point is that I can't think of any situation I'd actually want it.

Not sure what you mean. It's not an artificial restriction that we impose, it's a natural consequence of how arguments are passed in a function call. Most languages that support default values in function calls have this same limitation -- C++, Java, Python, etc.

Also, I guess I should say that named arguments don't "remove" the restriction so much as they provide a workaround by allowing you to order arguments however you like.

@fabianegli
Copy link

My questioning was aimed to find out wether this behaviour is desirable in this case - irrespective of wether it is introduced intentionally or not. The fact that it is prevalent in other languages seems to hint at it being sensible for some reasons that I am missing. As you say, in some cases it would allow a more organized workflows/processes definition by grouping related inputs independently of wether they have defaults or not. Because the named arguments already make arbitrary input orderings possible, I am not convinced that the effort to remove the order constraint would be worthwhile.

@pditommaso pditommaso force-pushed the master branch 2 times, most recently from 0d59b4c to b93634e Compare March 11, 2023 11:20
@pditommaso pditommaso force-pushed the master branch 2 times, most recently from 81f7cb7 to 8a43489 Compare August 20, 2023 20:13
@bentsherman bentsherman changed the title Support default values for process inputs Default values for process inputs Sep 15, 2023
@bentsherman bentsherman requested a review from a team as a code owner April 3, 2024 14:39
@bentsherman
Copy link
Member Author

Gonna fold this one into #4553 doing something like this:

process foo {
  input:
    List<Path> inputs
    String args = '--default-option'
  output:
    stdout
  script:
    """
    foo_cmd ${args} ${inputs}
    """
}

workflow {
  foo( file('*.foo') )
}

@bentsherman bentsherman deleted the 3507-process-input-default-values branch April 29, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optional input arguments, default value and named arguments.
3 participants