-
Notifications
You must be signed in to change notification settings - Fork 29
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
Fix throwing out wildtype hdr #16
Conversation
ice/classes/sanger_analysis.py
Outdated
""" | ||
Indels larger than min cutoff that overlap in size with donor are not considered as an edit proposal | ||
""" | ||
return self.donor_odn and self.HDR_OVERLAP_FILTER_CUTOFF < indel_size == self.donor_alignment.hdr_indel_size |
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 line is not clear. There are too many logical operations in the right hand side. Can you clean it up?
ice/classes/sanger_analysis.py
Outdated
for deletion_before in deletion_befores: | ||
for deletion_after in deletion_afters: | ||
if self._should_skip_proposal(deletion_before + deletion_after): | ||
continue |
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.
_should_skip_proposal treats deletions and insertions the same way?
self.donor_alignment.hdr_indel_size ==> if there is a 10bp deletion, is this value 10 or -10?
If there is a 10bp insertion, would it also throw out the 10bp deletion edit proposal?
Maybe have some more comments in the docstring for _should_skip_proposal
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.
Good catch. Don't see any reason you would want to throw out both the insertion and deletion. hdr_indel_size is negative for deletions (added a deletion only test case to hdr_indel_size).
Pull Request Test Coverage Report for Build 49
💛 - Coveralls |
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.
👍
Slight change to the proposal skipping where proposals less than a defined cutoff magnitude (3 right now) are NOT thrown out even if they are the same size as the insert.
Also, there was a +1 missing on the range to consider +20 inserts.
Lastly, adds a test for the substitution only case.