Skip to content
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.

Sign up for GitHub

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

FLOW_* parameters must be accompanied by FLOW_MODE #1980

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

FLOW_* parameters must be accompanied by FLOW_MODE #1980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
233cf9d
FLOW_* parameters must be accompanied by FLOW_MODE
dror27 Oct 13, 2024
a86772c
ensure legacyTest passes
dror27 Oct 13, 2024
8a61a69
ensure legacyTest runs
dror27 Oct 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions src/main/java/picard/sam/markduplicates/MarkDuplicates.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@
import htsjdk.samtools.*;
import htsjdk.samtools.DuplicateScoringStrategy.ScoringStrategy;
import htsjdk.samtools.util.*;
import org.broadinstitute.barclay.argparser.Argument;
import org.broadinstitute.barclay.argparser.ArgumentCollection;
import org.broadinstitute.barclay.argparser.CommandLineProgramProperties;
import org.broadinstitute.barclay.argparser.*;
import org.broadinstitute.barclay.help.DocumentedFeature;
import picard.PicardException;
import picard.cmdline.programgroups.ReadDataManipulationProgramGroup;
Expand Down Expand Up @@ -263,6 +261,8 @@ protected int doWork() {
// use flow based calculation helper?
if ( flowBasedArguments.FLOW_MODE ) {
calcHelper = new MarkDuplicatesForFlowHelper(this);
} else {
ensureNoFlowParametersSpecified();
}

reportMemoryStats("Start of doWork");
Expand Down Expand Up @@ -450,6 +450,29 @@ protected int doWork() {
return 0;
}

// make sure no FLOW__ parameter was set (except for FLOW_MODE)
private void ensureNoFlowParametersSpecified() {
if ( getCommandLineParser() instanceof CommandLineArgumentParser ) {
final CommandLineArgumentParser parser = (CommandLineArgumentParser)getCommandLineParser();
for (NamedArgumentDefinition def : parser.getNamedArgumentDefinitions()) {
final String name = def.getLongName();
if (name != "FLOW_MODE" && name.startsWith("FLOW_") && def.getHasBeenSet()) {
throw new PicardException(name + " specified, but no FLOW_MODE");
}
}
} else if ( getCommandLineParser() instanceof LegacyCommandLineArgumentParser ) {
final LegacyCommandLineArgumentParser parser = (LegacyCommandLineArgumentParser)getCommandLineParser();
if ( parser.getArgv() != null ) {
for ( final String name : parser.getArgv() ) {
if (!name.startsWith("FLOW_MODE") && name.startsWith("FLOW_")) {
throw new PicardException(name + " specified, but no FLOW_MODE");
}
}
}

}
}

/**
* package-visible for testing
*/
Expand Down
23 changes: 19 additions & 4 deletions src/test/java/picard/sam/markduplicates/MarkDuplicatesForFlowHelperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,16 @@ public void modify(final AbstractMarkDuplicatesCommandLineProgramTester tester)
}

@Test(dataProvider = "forFlowDataProvider")
public void testForFlowMDCall(final DuplicateScoringStrategy.ScoringStrategy scoringStrategy, final TestRecordInfo[] recInfos, final String[] params, TesterModifier modifier) {
public void testForFlowMDCall_WithFlowMode(final DuplicateScoringStrategy.ScoringStrategy scoringStrategy, final TestRecordInfo[] recInfos, final String[] params, TesterModifier modifier) {
testForFlowMDCall(scoringStrategy, recInfos, params, modifier, true);
}

@Test(dataProvider = "forFlowDataProvider")
public void testForFlowMDCall_WithoutFlowMode(final DuplicateScoringStrategy.ScoringStrategy scoringStrategy, final TestRecordInfo[] recInfos, final String[] params, TesterModifier modifier) {
testForFlowMDCall(scoringStrategy, recInfos, params, modifier, false);
}

private void testForFlowMDCall(final DuplicateScoringStrategy.ScoringStrategy scoringStrategy, final TestRecordInfo[] recInfos, final String[] params, TesterModifier modifier, final boolean withFlowMode) {

// get tester, build records
final AbstractMarkDuplicatesCommandLineProgramTester tester =
Expand Down Expand Up @@ -328,7 +337,9 @@ public void testForFlowMDCall(final DuplicateScoringStrategy.ScoringStrategy sco
}

// add parames, set flow order
tester.addArg("FLOW_MODE=true");
if ( withFlowMode ) {
tester.addArg("FLOW_MODE=true");
}
for ( final String param : params ) {
tester.addArg(param);
}
Expand All @@ -339,8 +350,12 @@ public void testForFlowMDCall(final DuplicateScoringStrategy.ScoringStrategy sco
modifier.modify(tester);
}

// run test
tester.runTest();
// run test. tests without flow mode should fail
if ( withFlowMode ) {
tester.runTest();
} else {
Assert.assertThrows(() -> tester.runTest());
}
}

@DataProvider(name ="getFlowSumOfBaseQualitiesDataProvider")
Expand Down
Loading