-
Notifications
You must be signed in to change notification settings - Fork 722
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
Re-write SRA download workflow to accept --ena_metadata_fields parameter #648
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.
LGTM 😄
Just a small question that probably is something I misunderstood.
@@ -4,6 +4,16 @@ | |||
======================================================================================== | |||
*/ | |||
|
|||
def valid_params = [ | |||
ena_metadata_fields : ['run_accession', 'experiment_accession', 'library_layout', 'fastq_ftp', 'fastq_md5'] |
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 got lost here, this means that only these 5 fields can be retrieved?
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.
Nope. The code here just checks that --ena_metadata_fields
contains those 5 fields as a minimum but it can contain as many others as long as they exist here
Was tricky figuring out how best to do this because if --ena_metadata_fields
is not specified then by default the fields here will be used but I didn't want to specify a long list list that in the nextflow_schema.json
so this was the workaround I came up with. I also manually curated the default list to remove any blank metadata fields so that accessions etc are next to each other in the final samplesheet so makes it easier to organise and visualise.
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.
Makes a lot of sense! Thanks for the explanation 😄
Closes #646