Make LocalizeReads Optional in GatherSampleEvidence #620
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
…eReads should be run with default value of true; if false, the task will be skipped.
…/bam from LocalizeReads if itis called only
…/bam from LocalizeReads if itis called only
… the original file
There was a problem hiding this comment.
This is looking good! I have a minor suggestion on the organization of the new input parameter. Also, can you describe your testing in more detail? (ie. did you test with run_localize_reads = true and false, and did you verify whether LocalizeReads was run/not run accordingly?)
epiercehoffman
approved these changes
Dec 8, 2023
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.
This pull request addresses issue #585. The LocalizeReads task was introduced to GatherSampleEvidence to simplify cases for CRAMs sourced from requester-pays and multi-regional buckets to simplify the code, but could be skipped in some cases for some cost and time savings. An optional boolean variable set to a default value of true to run LocalizeReads was added. When set to false, the LocalizeReads task is skipped.
This passed validation with both womtool and cromshell against the reference panel. The input in the json was changed to both True and False and ran as expected in both scenarios.