Add prefix to output name #3
Merged
+18
−13
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrisamiller
approved these changes
Jun 26, 2019
susannasiebert
requested changes
Jun 26, 2019
bam_readcount_helper.py
Outdated
| @@ -96,14 +101,14 @@ def filter_sites_in_hash(region_list, bam_file, ref_fasta, sample, output_dir, i | |||
|
|
|||
| if len(rc_for_snp.keys()) > 0: | |||
| region_file = generate_region_list(rc_for_snp) | |||
| filter_sites_in_hash(region_file, bam_file, ref_fasta, sample, output_dir, False, min_mapping_qual, min_base_qual) | |||
| filter_sites_in_hash(region_file, bam_file, ref_fasta, sample, output_dir, False, prefix, min_mapping_qual, min_base_qual) | |||
There was a problem hiding this comment.
Instead of changing the parameters to filter_sites_in_hash I propose that you create a variable prefixed_sample and pass that along instead of the sample name. To make that work, on line 46-49 you would do:
if prefix == 'NOPREFIX':
prefixed_sample = sample
else:
prefixed_sample = '_'.join([prefix, sample])
You can then also use prefixed_sample to set your output_file below.
There was a problem hiding this comment.
This makes sense as better coding. I make the change as requested. It's been tested on real case with and without 'NOPREFIX', and output tsv files are generated as expected.
susannasiebert
approved these changes
Jun 27, 2019
|
That looks much cleaner. +1 from me. I still agree with @chrisamiller that we should refactor this script to use argparse or similar but that is outside of the scope of this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For the case of aml trio pipeline, we need run bam_readcount on followup (day 30) samples with the use of tumor detect_variant vcf. There is a need to differentiate bam_readcount output tsv from "followup" and "tumor" samples. @johnegarza Can you review this ?