Removing MarkDuplicatesGATK #5166
Conversation
… Picard implemenation and MarkDuplicatesGATK
|
I did not remove EstimateLibraryComplexityGATK, It would be easy to do as part of this pr and would mean most of the markDuplicatesPackage can be removed entirely. |
|
I'd kill EstimateLibraryComplexity while you're at it. |
There was a problem hiding this comment.
@jamesemery One comment about moving tests, I'm not sure they all should move up since ostensibly those top level tests should be general to all markduplicates ( and we would like to eventually merge with the picard tests directly...)
I would delete EstimateLibraryComplexityGATK as well while we're here...
| } | ||
|
|
||
| @Test(dataProvider = "readNameData") | ||
| public void testOpticalDuplicateClusterSamePositionNoOpticalDuplicates(final String readName1, final String readName2) { |
There was a problem hiding this comment.
Why move all these tests up to AbstractMarkDuplicatesCommandLineTest? It doesn't seem like things like testHashCollisionHandling and testMarkDuplicatesSparkIntegrationTestLocal` belong there.
|
@lbergelson I agree with you about moving tests up, though I would make the point that there were many tests that got pushed down in the first place because they would involve fixing bugs in MarkDuplicatesGATK that were already fixed in MarkDuplicatesSpark, I will audit the ones I did and didn't push up so i'm more confident there is a reason to have tests pushed down or not. |
|
@lbergelson back to you |
In light of the newly updated MarkDuplicatesSpark which is finally reaching the point where we trust it, we have evaluated that the work of maintaining both this tool and MarkDuplicatesSpark was too great considering how little code they could directly reuse relative to eachother.
Resolves #4896 #3705