-
Notifications
You must be signed in to change notification settings - Fork 596
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
Updating MarkDuplicatesSpark tiebreaking to reflect changes in picard #5011
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5011 +/- ##
================================================
+ Coverage 60.157% 86.078% +25.921%
- Complexity 12768 29356 +16588
================================================
Files 1095 1783 +688
Lines 64608 134963 +70355
Branches 10395 15463 +5068
================================================
+ Hits 38866 116173 +77307
+ Misses 21504 13350 -8154
- Partials 4238 5440 +1202
|
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.
@jamesemery Documentation comments. Looks good otherwise. Do you have any sense if the short circuiting for single pairs saves us any time?
import picard.sam.util.PhysicalLocation; | ||
|
||
/** | ||
* A common interface for holding the fields in PhysicalLocation |
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 isn't an interface
|
||
// Methods for OpticalDuplicateFinder.PhysicalLocation | ||
@Override | ||
public short getReadGroup() { return this.readGroupIndex; } |
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.
it's weird that these read group isn't transient but the others are, should you javadoc that?
Tests ported from picard: broadinstitute/picard#1195
Fixes #4707