Repository navigation
Use Nextflow's plain values of Channel and Value params - #235
Draft
pinin4fjords wants to merge 1 commit into
Draft
pinin4fjords wants to merge 1 commit into
pinin4fjords wants to merge 1 commit into
Conversation
…e params Nextflow 26.10.0 resolves each typed `Channel` and `Value` param to the value it was created from (`ParamsMap.toPlainMap()`, nextflow-io/nextflow#7759), so validation and the params summary use that instead of looking the value up in the config. The tests use typed params blocks. Requires Nextflow 26.10.0, which is not released yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
|
Thanks already! We can come back to this after the release next week 🎉 |
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Important
Waiting for a Nextflow release. The Nextflow side (nextflow-io/nextflow#7759, closing nextflow-io/nextflow#7758) is merged, but not in a release yet. This PR sets
nextflowVersion = '26.10.0', so it won't build until Nextflow 26.10.0 is out. After that, mergingmasterin (and adjusting the version if the release number differs) should be all it needs.@nvnieuwk, for your awareness: this is the follow-up to the TODO from #230.
What it does
Nextflow now gives the value each typed
ChannelandValueparam was created from throughsession.params.toPlainMap(). This PR uses that invalidateParameters()and the params summary, and removes nf-schema's own lookup in the config (replaceDataflowParamsand its helpers inCommon.groovy). Typed params now reach the schema in their declared types and with their script defaults, which the config lookup could not give.The dataflow tests from #230 are rewritten to use real typed
paramsblocks, and the mocked-session spec (ValidateDataflowParamsTest) is replaced by them.Open questions
toPlainMap()exists and fall back tosession.params(no older Nextflow release has typedChannel/Valueparams).paramsin the script (params.x = channel.of(...), not declared in aparamsblock) was handled by Validate and print the value of Channel and Value params instead of hanging #230's config lookup but isn't covered bytoPlainMap(). Is it fine to drop that case?Testing
All 161 tests pass against a local build of Nextflow master (3e6b01d35, which includes nextflow-io/nextflow#7759) published as 26.10.0. With
session.paramsin place ofsession.params.toPlainMap(), the new tests fail, several by hitting their timeout.🤖 Generated with Claude Code