Repository navigation
Validate and print the value of Channel and Value params instead of hanging #230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
pinin4fjords
merged 15 commits into
nextflow-io:master
from
pinin4fjords:fix/dataflow-params-validation
Oct 5, 2026
Merged
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
d7d98d0
fix(parameters): validate the value of Channel and Value params inste…
pinin4fjords fca4bf6
test(parameters): cover the command line path, precedence and typed C…
pinin4fjords ccbd62c
fix(summary): leave dataflow params out of the parameter summary
pinin4fjords e7546d9
fix(parameters): read the dataflow param value from the config params…
pinin4fjords 52c4e0f
docs: use the next minor version for the CHANGELOG entry
pinin4fjords 545b4a2
fix(parameters): detect dataflow params by class instead of package name
pinin4fjords 2d38836
chore: bump the version to 3.1.0
pinin4fjords 123b70d
refactor(parameters): replace the dataflow params in the Map converter
pinin4fjords cb92ef6
Merge upstream master
pinin4fjords 163165a
docs: describe the parameters that are a Channel or a Value
pinin4fjords 8a009f9
Merge upstream master
pinin4fjords d945c9a
test(parameters): cast the command line value of a dataflow param
pinin4fjords df0a0cb
docs: link the replacement of dataflow params to its planned successor
pinin4fjords e86c650
docs: word the planned successor of the dataflow param replacement as…
pinin4fjords e804b6b
docs: link the dataflow param replacement to the Nextflow issue
pinin4fjords File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
172 changes: 172 additions & 0 deletions
172
src/test/groovy/nextflow/validation/ValidateDataflowParamsTest.groovy
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,172 @@ | ||
| /* groovylint-disable LineLength, MethodName */ | ||
| package nextflow.validation | ||
|
|
||
| import groovy.transform.CompileDynamic | ||
| import groovyx.gpars.dataflow.DataflowQueue | ||
| import groovyx.gpars.dataflow.DataflowVariable | ||
| import nextflow.Session | ||
| import nextflow.dataflow.ChannelImpl | ||
| import nextflow.dataflow.ValueImpl | ||
| import nextflow.validation.config.ValidationConfig | ||
| import nextflow.validation.exceptions.SchemaValidationException | ||
| import nextflow.validation.parameters.ParameterValidator | ||
| import spock.lang.Specification | ||
| import spock.lang.Timeout | ||
|
|
||
| import java.nio.file.Path | ||
|
|
||
| /** | ||
| * Validation of params that hold a dataflow value (a typed `Channel` or `Value` param), where the value | ||
| * the param was created from is validated in place of the dataflow object. Nextflow keeps that value in | ||
| * the params scope of the config, which also holds the values given on the command line and in a params | ||
| * file. The session is mocked so that the config params can be set directly. | ||
| */ | ||
| @CompileDynamic | ||
| @Timeout(60) | ||
| class ValidateDataflowParamsTest extends Specification { | ||
|
|
||
| private static final String SCHEMA = 'src/testResources/nextflow_schema.json' | ||
| private static final String NESTED_SCHEMA = 'src/testResources/nextflow_schema_nested_parameters.json' | ||
|
|
||
| void 'should accept a valid value of a dataflow param'() { | ||
| given: | ||
| Session session = mockSession(topLevelParams(new DataflowVariable()), [input: 'src/testResources/correct.csv']) | ||
|
|
||
| when: | ||
| validate(session, SCHEMA) | ||
|
|
||
| then: | ||
| noExceptionThrown() | ||
| } | ||
|
|
||
| void 'should reject an invalid value of a dataflow param'() { | ||
| given: | ||
| Session session = mockSession(topLevelParams(new DataflowVariable()), [input: 'src/testResources/correct.txt']) | ||
|
|
||
| when: | ||
| validate(session, SCHEMA) | ||
|
|
||
| then: | ||
| SchemaValidationException error = thrown(SchemaValidationException) | ||
| error.message.contains('--input (src/testResources/correct.txt)') | ||
| } | ||
|
|
||
| void 'should accept a valid value of a Value param'() { | ||
| given: | ||
| Session session = mockSession(topLevelParams(new ValueImpl(new DataflowVariable())), [input: 'src/testResources/correct.csv']) | ||
|
|
||
| when: | ||
| validate(session, SCHEMA) | ||
|
|
||
| then: | ||
| noExceptionThrown() | ||
| } | ||
|
|
||
| void 'should reject an invalid value of a Value param'() { | ||
| given: | ||
| Session session = mockSession(topLevelParams(new ValueImpl(new DataflowVariable())), [input: 'src/testResources/correct.txt']) | ||
|
|
||
| when: | ||
| validate(session, SCHEMA) | ||
|
|
||
| then: | ||
| SchemaValidationException error = thrown(SchemaValidationException) | ||
| error.message.contains('--input (src/testResources/correct.txt)') | ||
| } | ||
|
|
||
| void 'should accept a valid value of a Channel param'() { | ||
| given: | ||
| Session session = mockSession(topLevelParams(new ChannelImpl(new DataflowQueue())), [input: 'src/testResources/correct.csv']) | ||
|
|
||
| when: | ||
| validate(session, SCHEMA) | ||
|
|
||
| then: | ||
| noExceptionThrown() | ||
| } | ||
|
|
||
| void 'should reject an invalid value of a Channel param'() { | ||
| given: | ||
| Session session = mockSession(topLevelParams(new ChannelImpl(new DataflowQueue())), [input: 'src/testResources/correct.txt']) | ||
|
|
||
| when: | ||
| validate(session, SCHEMA) | ||
|
|
||
| then: | ||
| SchemaValidationException error = thrown(SchemaValidationException) | ||
| error.message.contains('--input (src/testResources/correct.txt)') | ||
| } | ||
|
|
||
| void 'should accept a valid value of a dataflow param when the command line values are not cast'() { | ||
| given: | ||
| Session session = mockSession(topLevelParams(new ValueImpl(new DataflowVariable())), [input: 'src/testResources/correct.csv']) | ||
|
|
||
| when: | ||
| validate(session, SCHEMA, [cast_cli_params: false]) | ||
|
|
||
| then: | ||
| noExceptionThrown() | ||
| } | ||
|
|
||
| void 'should reject an invalid value of a dataflow param when the command line values are not cast'() { | ||
| given: | ||
| Session session = mockSession(topLevelParams(new ValueImpl(new DataflowVariable())), [input: 'src/testResources/correct.txt']) | ||
|
|
||
| when: | ||
| validate(session, SCHEMA, [cast_cli_params: false]) | ||
|
|
||
| then: | ||
| SchemaValidationException error = thrown(SchemaValidationException) | ||
| error.message.contains('--input (src/testResources/correct.txt)') | ||
| } | ||
|
|
||
| void 'should accept a valid value of a dataflow value nested in a record param'() { | ||
| given: | ||
| Session session = mockSession( | ||
| [map: [is: [so: [deep: new ValueImpl(new DataflowVariable())]]]], | ||
| [map: [is: [so: [deep: true]]]] | ||
| ) | ||
|
|
||
| when: | ||
| validate(session, NESTED_SCHEMA) | ||
|
|
||
| then: | ||
| noExceptionThrown() | ||
| } | ||
|
|
||
| void 'should reject an invalid value of a dataflow value nested in a record param'() { | ||
| given: | ||
| Session session = mockSession( | ||
| [map: [is: [so: [deep: new ValueImpl(new DataflowVariable())]]]], | ||
| [map: [is: [so: [deep: 'maybe']]]] | ||
| ) | ||
|
|
||
| when: | ||
| validate(session, NESTED_SCHEMA) | ||
|
|
||
| then: | ||
| SchemaValidationException error = thrown(SchemaValidationException) | ||
| error.message.contains('--map.is.so.deep') | ||
| } | ||
|
|
||
| private Session mockSession(Map params, Map configParams) { | ||
| Session session = Mock(Session) | ||
| session.params >> params | ||
| session.config >> [params: configParams] | ||
| session.baseDir >> Path.of('.').toAbsolutePath() | ||
| return session | ||
| } | ||
|
|
||
| private void validate(Session session, String schema, Map options = [:]) { | ||
| ValidationConfig config = new ValidationConfig([monochromeLogs: true], session) | ||
| new ParameterValidator(config).validateParametersMap( | ||
| [parameters_schema: Path.of(schema).toAbsolutePath().toString()] + options, | ||
| session | ||
| ) | ||
| } | ||
|
|
||
| private Map topLevelParams(Object input) { | ||
| return [input: input, outdir: 'src/testResources/testDir'] | ||
| } | ||
|
|
||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you directly add a converter on a Dataflow type here instead? (I'm not too familiar with those data types but that seems the most logical to me?)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honest disclaimer: this is beyond me, so I worked through it with Claude (Claude Code). This also answers your other comment about skipping these params.
A converter on the dataflow types can't do it, for two reasons. It only gets the object and its key name, not the full path, so for nested params it can't tell
rnaseq.inputfromdiffab.input. And the object itself doesn't hold the original value in a usable form: at this point aChannel/Valueparam is still a live dataflow object, and reading it can block. That's why the converter is on theMap: the generator sees the params map before it converts the entries, so the dataflow params can be replaced there.The original value, such as the samplesheet path, is in
session.config.params(which includes values from the CLI, params files and config), so we resolve it from there by its full path. Returningnullfrom a converter isn't a way to remove an entry either, because null filtering happens before converters are applied.Skipping these params wouldn't be equivalent. The original value of a
Channelparam is usually a file path, and that's what the schema validates (format: file-path, the samplesheet schema, etc.). Omitting it would bypass that validation, and a required param would be reported as missing.This may get simpler if Nextflow gets a core way to give these plain values (nextflow-io/nextflow#7758; I've proposed one, also with Claude). Typed
Channel/Valueparams from nextflow-io/nextflow#7213 aren't in a released Nextflow yet either, so I'd keep this implementation for now and revisit if that happens.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the honesty and now I get why you did it like this. Fine by me to use this implementation now and convert to a more solid approach in the future. Could you add a comment that links to your PR/issue in Nextflow and this PR so we can refer back there in the future?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Added in e804b6b, as a TODO on
replaceDataflowParamsinCommon.groovy(both the validation and the summary use it), linking nextflow-io/nextflow#7758 and this PR.I also merged master after #234 went in (8a009f9). The CLI cast is now a pass before the generator, so the dataflow replacement had to move out of the
Mapconverter into a pass just before it, otherwise a dataflow param given on the command line would not get cast. There is a test for that in d945c9a.