-
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
Deleted some unused genotyping code #6354
Conversation
@vruano You should take a look and see if any of these chagnes are going to conflict with your HMM changes. I know you changed many of those classes in your branch. |
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.
@davidbenjamin I saw a few things that could use additional cleanup. Seems sane though.
@@ -389,15 +382,15 @@ else if (matchingStart.size() == 1) { | |||
* Creates a UnifiedArgumentCollection with appropriate values filled in from the arguments in this walker |
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 comment still talks about UnifiedArgumentCollection
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.
fixed
@@ -10,15 +10,15 @@ | |||
* used only by the HaplotypeCaller for its isActive() determination. Should not be used for | |||
* any other purpose! | |||
*/ | |||
public final class MinimalGenotypingEngine extends GenotypingEngine<UnifiedArgumentCollection> { | |||
public final class MinimalGenotypingEngine extends GenotypingEngine<StandardCallerArgumentCollection> { | |||
|
|||
/** | |||
* Creates a new unified genotyping given the UG configuration parameters and the targeted set of samples |
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.
all these comments talk about the UG configuration which is confusing and outdated
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.
fixed
@@ -307,19 +307,18 @@ private void initializeSamples() { | |||
|
|||
private void initializeActiveRegionEvaluationGenotyperEngine() { | |||
// create a UAC but with the exactCallsLog = null, so we only output the log for the HC caller itself, if requested |
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.
UAC
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.
fixed
@@ -495,7 +494,7 @@ public ActivityProfileState isActive( final AlignmentContext context, final Refe | |||
// TODO: now that old qual is gone, do we still need this? |
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 todo still relevant?
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.
PR #6343 addressed it, so the TODO will be gone after rebasing.
final UnifiedArgumentCollection uac = new UnifiedArgumentCollection(); | ||
uac.genotypeArgs = new GenotypeCalculationArgumentCollection(); | ||
final StandardCallerArgumentCollection standardArgs = new StandardCallerArgumentCollection(); | ||
standardArgs.genotypeArgs = new GenotypeCalculationArgumentCollection(); |
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.
I don't think this needs to be set here anymore since it was just constructed with the same thing.
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.
fixed
uac.genotypeArgs = new GenotypeCalculationArgumentCollection(genotypeArgs); | ||
return new MinimalGenotypingEngine(uac, SAMPLES); | ||
final StandardCallerArgumentCollection standardArgs = new StandardCallerArgumentCollection(); | ||
standardArgs.genotypeArgs = new GenotypeCalculationArgumentCollection(genotypeArgs); |
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.
likewise
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.
fixed
Woops, messed up assigning people to this pr. Sorry @davidangb |
@lbergelson back to you |
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.
👍
@lbergelson This stuff is all dead code. Would you care to review?