-
Notifications
You must be signed in to change notification settings - Fork 673
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
[PROOF OF PRINCIPLE] Rename publish_by_id to publish_by_meta and add flexibility of taking a list of ids #590
Conversation
|
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.
Awesome solution! 💥
path = path instanceof String ? path : id | ||
path_list.add(path) | ||
} | ||
} | ||
} |
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.
What happens when the id is not String for example: publish_by_meta = [1, 2, 3] or when the meta[id] is true/false?
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.
If the key
is not a String
it will be ignored via the statement below:
if (args.meta && key instanceof String) {
for (id in publish_id_list) { | ||
if (args.meta && id instanceof String) { | ||
def path = args.meta.containsKey(id) ? args.meta[id] : id | ||
path = path instanceof String ? path : id |
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 like all non-String path would be ignored. Would you try to convert the path to String here? Is it possible to treat boolean value to be yes/no and append it to the end of the key? Also, can you replace id to key in the for loop so that it won't be confused between meta['id'] and meta[id]?
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.
Good idea! Done 1ab8199
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.
Please don't forget to update the modules.conf header part:
/*
-
- Config file for defining DSL2 per module options
-
- Available keys to override module options:
-
args = Additional arguments appended to command in module.
-
args2 = Second set of arguments appended to command in module (multi-tool modules).
-
publish_dir = Directory to publish results.
-
publish_by_meta = Groovy list of keys available in meta map to append as directories to "publish_dir" path
-
If publish_by_meta = true - Value of ${meta['id']} is appended as a directory to "publish_dir" path
-
If publish_by_meta = ['id', 'custompath'] - If "id" is in meta map and "custompath" isn't then "${meta['id']}/custompath/"
-
is appended as a directory to "publish_dir" path
-
If publish_by_meta = false / null - No directories are appended to "publish_dir" path
-
publish_files = Groovy map where key = "file_ext" and value = "directory" to publish results for that file extension
-
The value of "directory" is appended to the standard "publish_dir" path as defined above.
-
If publish_files = null (unspecified) - All files are published.
-
If publish_files = false - No files are published.
-
suffix = File name suffix for output files.
*/
Added in nf-core/modules#423 and #591 |
PR checklist
nf-core lint .
).nextflow run . -profile test,docker
).docs/usage.md
is updated.docs/output.md
is updated.CHANGELOG.md
is updated.README.md
is updated (including new tool citations and authors/contributors).