-
Notifications
You must be signed in to change notification settings - Fork 297
Add documentation for non-argument module inputs #4210
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
5091b7a
a3c89df
0b6d36f
93b093c
443237e
ee660ec
6f977eb
e602cc2
7c91833
79ab1ee
ab8e427
5d1655f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,19 @@ When one and only one of multiple argument are required: | |
|
|
||
| ::: | ||
|
|
||
| ## Non-argument `val` channel inputs | ||
|
|
||
| Input channel `val` declarations MAY be used to control behaviours of the module that cannot be expressed with arguments of the underlying tool. | ||
|
|
||
| - If a module implements multiple subcommands of a tool, the subcommand SHOULD be provided through a channel. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't the rule that subcommands should be their own module?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generally yes, but there are some modules where a subcommand or subsubcommand is passed as an input (specifically, per Codex and confirmed manually:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would say unnoticed deviations
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll wait a bit for differential opinions, but if others agree with you I'll remove it since I don't want to start a civil war here
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I agree, this sounds like things that slipped through review. The only case I can imagine this being valid if it all subcommands share exactly the same options and arguments ,(so they apply to every subcommand), but then this would be very unlikely as they you might as well just control this via another argument.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, changed it. I used SHOULD, since there might be very specific cases where this makes sense. |
||
| - If a module can output arbitrary or numerous file formats, the output format SHOULD be provided through a channel. The module SHOULD NOT perform bespoke string manipulation on input paths. | ||
|
itrujnara marked this conversation as resolved.
Outdated
|
||
| :::info{title="Rationale" collapse} | ||
| Modules can encounter numerous input name scenarios. Custom string operations necessarily make assumptions about the name of the file (for example that the name of a compressed file has at least two dots). Providing an explicit format input returns full control to the pipeline developer and reduces the risk of unexpected behaviour. | ||
| ::: | ||
| - If a module contains an optional pipe (for example: compression, sorting), the pipe SHOULD be controlled with a Boolean input channel. | ||
|
mahesh-panchal marked this conversation as resolved.
muffato marked this conversation as resolved.
muffato marked this conversation as resolved.
Comment on lines
+72
to
+75
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK the more I read about this, this feels mostly more about how to define output names, in which case I think the title of the section should be changed accordingly and rephrased to be more active of what you SHOULD do (a lot of this seems to be more defensive). E.g., a seciton on output naming would be more like:
Or something like that
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also technically this also overlaps with this section actually: https://nf-co.re/docs/specifications/components/modules/naming-conventions#command-file-output-naming
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not meant to be a section on outputs, but rather on inputs. The current documentation largely suggests that every input to the module should be an input to the tool, which is clearly not the case. The involvement of output names is due to the drama of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basically what I want here is to move ideas from random Slack threads to the documentation
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't think this is necessarily true: https://nf-co.re/docs/specifications/components/modules/input-output-options#required-val-channel-inputs ('essential for executuion of the tool' does not necessarily reuqire this to be an input to the tool itself - but we could make this clearer if yo uwanted?) |
||
|
|
||
| Non-standard `ext` fields (example: `ext.suffix`) SHOULD NOT be used to control module behaviour. | ||
|
|
||
| ## Output channel emissions | ||
|
|
||
| Named file extensions MUST be emitted for ALL output channels. | ||
|
|
||
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.
Is this what you mean?
At the moment reading this I'm not sure how this differs from the much more concise point 1 of the previous section (although we could have a an example):
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.
This is not quite what I mean. This section aims to codify existing practice for inputs like
val action,val compress, andval out_fmt, which are common in modules, but lack appropriate documentation. These input are not really "an argument to the tool", but are still an important argument to the module.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.
Ah I think I'm starting to follow now.
I guess then just a single additional sentence of:
'This can include parameters to control execution of the tool, outside of the tool parameters itself - such as an optional post-execution compression step (to adhere to nf-core specification XYZ)' or something like this?