-
Notifications
You must be signed in to change notification settings - Fork 635
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
Include all processes in inspect command #5580
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Ben Sherman <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
✅ Deploy Preview for nextflow-docs-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Yesss! 🙏🏻
Signed-off-by: Ben Sherman <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
// don't execute entry workflow if preview action (i.e. inspect command) is specified | ||
if( previewAction ) | ||
scriptParser.setModule(true) |
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.
Since the previewAction was created for the inspect command and isn't used for anything else, I use it here to disable the entry workflow execution
Setting the module flag will register the process definitions without running the entry workflow:
nextflow/modules/nextflow/src/main/groovy/nextflow/script/BaseScript.groovy
Lines 159 to 162 in 53d6c61
final result = runScript() | |
if( meta.isModule() ) { | |
return result | |
} |
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.
Then I'd say to get rid of previewAction
(since it's useless) and use instead ContainerInspectMode that's more explicit about the intent.
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.
Okay I tried using the container inspect mode. Don't see a clean way to remove the previewAction altogether as it's a convenient way to pass the callback into the script runner.
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.
Looks good! made a couple of minor comments
// don't execute entry workflow if preview action (i.e. inspect command) is specified | ||
if( previewAction ) | ||
scriptParser.setModule(true) |
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.
Then I'd say to get rid of previewAction
(since it's useless) and use instead ContainerInspectMode that's more explicit about the intent.
modules/nextflow/src/main/groovy/nextflow/script/ProcessDef.groovy
Outdated
Show resolved
Hide resolved
Signed-off-by: Ben Sherman <[email protected]>
I found a way to improve the
inspect
command without using the new script parser -- I just look at all process definitions across all included scripts, regardless of whether they are ever called.All processes are included, even if a process is defined but never called by the workflow. Static analysis would be required to extract the process calls from the workflow.
The process "base name" is used instead of the fully qualified name, and aliases are not included. Static analysis would be required to get the fully qualified name. But I figured the base name is fine because you shouldn't need to specify different containers for different instances of the same process.
I went ahead with this approach because nf-core needs this functionality and I don't know how long it will take to incorporate the new script parser.
Here's the output for rnaseq: