-
Notifications
You must be signed in to change notification settings - Fork 1
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
Processes abstracted into a shared file #28
Conversation
workflows/cath-shared.nf
Outdated
params.cath_af_cli = 'cath-af-cli' | ||
params.cath_version = 'v4_3_0' | ||
|
||
params.publish_dir = "$workflow.launchDir/cath-test-hpc-${params.dataset_name}" |
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 probably workflow specific - maybe move this one back to the workflow?
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.
Actually, we could just make it less workflow specific:
params.publish_dir = "$workflow.launchDir/results-${params.dataset_name}"
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.
|Made that change and added results-*/ to gitignore
workflows/cath-shared.nf
Outdated
path uniprot_id_file | ||
|
||
output: | ||
path params.af_manifest_fn |
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.
I shouldn't have used a global variable here - creates an unnecessary dependency.
This could be any filename - and it's probably better that the process takes care of this internally:
output:
path 'manifest.txt'
"""
... > manifest.txt
"""
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.
Done and checked in
workflows/cath-shared.nf
Outdated
path uniprot_domain_id | ||
|
||
output: | ||
path params.uniprot_ids_csv_fn |
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.
Again, we aren't publishing the output file, so we don't need to use the global filename - probably better to replace this with 'uniprot_ids.txt'
(and change the script to reflect)
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.
(sorry, I know this was my code!)
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.
Nice, thanks. Added a few suggestions - mainly to change my code(!). Welcome to merge whenever you're done.
Closes #25