-
Notifications
You must be signed in to change notification settings - Fork 597
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
Modified Fragment creation for mutect2 to not fail for supplementary reads #6327
Modified Fragment creation for mutect2 to not fail for supplementary reads #6327
Conversation
…e that a Fragment is only supported by supplementary and/or secondary and/or duplicate reads Fixes #6310
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 good!
@@ -11,6 +11,7 @@ | |||
import java.util.Arrays; | |||
import java.util.Collections; | |||
import java.util.List; | |||
import java.util.Optional; |
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.
Is this new import used?
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.
Oops, I forgot to remove that before committing. I'll take that out
hi @KevinCLydon, @davidbenjamin , thanks for this fix. I am preparing a custom version of 4.1.4.1 with just this patch on top of it. One question I had is: is this a safe patch to have on its own, or does it need to go in tandem with any other patch to form a proper release (and not break anything else)? Given this fix is now ~7 months old, I'm hoping insiders can maybe tell me if any other patch ended up being needed after this one, either to deal with the same issue, or to deal with any new issues introduced by it (if any). |
@pieterlukasse I don't know if this is an appropriate solution. |
thanks @fleharty . I can confirm that using |
@fleharty I looked up other tickets for this extra --read-filter and found this #6230 (comment) . This comment and following comments lead me to think there might be an issue with this --read-filter that was only solved in 4.1.5.0? Or has --read-filter issue also been solved in 4.1.4.1? |
I think I understand it now: #6240 did fix the |
@pieterlukasse Almost correct: #6240 partially solved the issue but left an edge case that #6327 fixed. The 4.1.5.0 release is the only one guaranteed to work, though 4.1.4.1 works for the great majority of samples. |
Modified the fragment creation code to not throw an exception in the case that there are more than 2 reads supporting a fragment and they are all supplementary, secondary, and/or duplicates. Instead, it will just pick one of those reads to use.