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

Jump to bottom

Protection against using the wrong sample alias which produces zero L… #1242

Merged
merged 6 commits into from
Oct 23, 2018
Merged

Protection against using the wrong sample alias which produces zero L… #1242

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
232addf
Protection against using the wrong sample alias which produces zero L…
Oct 17, 2018
3eeec7d
- oops fixed failing tests
Oct 17, 2018
318cbf5
- add TWR block
Oct 23, 2018
f92ff60
- responding to review
Oct 23, 2018
427a60f
- responding to review
Oct 23, 2018
b024781
- responding to review
Oct 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/main/java/picard/fingerprint/CheckFingerprint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ protected int doWork() {
final MetricsFile<FingerprintingSummaryMetrics, ?> summaryFile = getMetricsFile();
final MetricsFile<FingerprintingDetailMetrics, ?> detailsFile = getMetricsFile();

boolean anyNonzeroLod = false;
for (final FingerprintResults fpr : results) {
final MatchResults mr = fpr.getMatchResults().first();

Expand Down Expand Up @@ -365,11 +366,22 @@ protected int doWork() {

summaryFile.addMetric(metrics);
log.info("Read Group: " + metrics.READ_GROUP + " / " + observedSampleAlias + " vs. " + metrics.SAMPLE + ": LOD = " + metrics.LOD_EXPECTED_SAMPLE);
if (metrics.LOD_EXPECTED_SAMPLE!=0) {
yfarjoun marked this conversation as resolved.
Show resolved Hide resolved
anyNonzeroLod |= true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bitwise |= here isn't necessary. The RHS of the statement is always true, so a simple = would suffice. Also, if we don't care about writing the metrics files in the case of this error, we can just output the error log from below and return 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"...and return 1" in what case? I am erroring out when I fail to find a non-zero LOD for all the FPs in the loop....so I need to wait until I'm out of the loop

}
}

summaryFile.write(outputSummaryMetricsFile);
detailsFile.write(outputDetailMetricsFile);


if (!anyNonzeroLod){
yfarjoun marked this conversation as resolved.
Show resolved Hide resolved
yfarjoun marked this conversation as resolved.
Show resolved Hide resolved
log.error("No non-zero results found. This is likely an error. " +
"Probable cause: EXPECTED_SAMPLE (if provided) or the sample name from INPUT (if EXPECTED_SAMPLE isn't provided)" +
"isn't a sample in GENOTYPES file.");
return 1;
}

return 0;
}

Expand Down
49 changes: 37 additions & 12 deletions src/test/java/picard/fingerprint/CheckFingerprintTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,31 @@ public void testBaseOutput() {
RESULT_EXAMPLE_DETAIL));
}

@Test
public void testMismatchingSamples() {
String[] args = new String[]{
"I=" + NA12891_r1_sam,
"O=" + TEST_OUTPUT,
"G=" + TEST_GENOTYPES_VCF1,
"H=" + SUBSETTED_HAPLOTYPE_DATABASE_FOR_TESTING
};

Assert.assertEquals(runPicardCommandLine(args), 1);
}

@Test
public void testMismatchingSamples2() {
String[] args = new String[]{
"I=" + TEST_INPUT_VCF1,
"O=" + TEST_OUTPUT,
"G=" + TEST_GENOTYPES_VCF1,
"EXPECTED_SAMPLE_ALIAS=TEST123",
"H=" + SUBSETTED_HAPLOTYPE_DATABASE_FOR_TESTING
};

Assert.assertEquals(runPicardCommandLine(args), 1);
}

@Test
public void testSummaryAndDetailOutputs() {
String[] args = new String[]{
Expand Down Expand Up @@ -173,36 +198,36 @@ public void setup() throws IOException {
@DataProvider(name = "samsToFingerprint")
Object[][] samsToFingerprint() {
return new Object[][]{
{NA12891_r1_sam, na12891_fp},
{NA12892_r1_sam, na12891_fp},
{NA12891_r1_sam, na12891_fp, 0},
{NA12892_r1_sam, na12891_fp, 1},
};
}

@DataProvider(name = "vcfsToFingerprint")
Object[][] vcfsToFingerprint() {
return new Object[][]{
{NA12891_named_NA12892_vcf, na12892_fp},
{NA12892_1_vcf, na12892_fp},
{NA12891_named_NA12892_vcf, na12892_fp, 0},
{NA12892_1_vcf, na12892_fp, 0},
};
}

@Test(dataProvider = "samsToFingerprint")
void testCheckFingerprintSam(File file, File genotypes) throws IOException {
tester(false, file, genotypes);
void testCheckFingerprintSam(final File file, final File genotypes, final int expectedRetVal) throws IOException {
tester(false, file, genotypes, expectedRetVal);

}

@Test(dataProvider = "vcfsToFingerprint")
void testCheckFingerprintVcf(File file, File genotypes) throws IOException {
tester(false, file, genotypes);
void testCheckFingerprintVcf(final File file, final File genotypes, final int expectedRetVal) throws IOException {
tester(false, file, genotypes, expectedRetVal);
}

@Test(dataProvider = "samsToFingerprint")
void testCheckFingerprintNoRg(File file, File genotypes) throws IOException {
tester(true, file, genotypes);
void testCheckFingerprintNoRg(final File file, final File genotypes, final int expectedRetVal) throws IOException {
tester(true, file, genotypes, expectedRetVal);
}

private File tester(boolean ignoreRG, File file, File genotypes) throws IOException {
private File tester(boolean ignoreRG, final File file, final File genotypes, final int expectedRetVal) throws IOException {
final List<String> args = new ArrayList<>();
final File outputSummary = File.createTempFile("fingerprint", "summary_metrics");
outputSummary.deleteOnExit();
Expand All @@ -216,7 +241,7 @@ private File tester(boolean ignoreRG, File file, File genotypes) throws IOExcept
args.add("SUMMARY_OUTPUT=" + outputSummary.getAbsolutePath());
args.add("DETAIL_OUTPUT=" + outputDetail.getAbsolutePath());

Assert.assertEquals(runPicardCommandLine(args), 0);
Assert.assertEquals(runPicardCommandLine(args), expectedRetVal);

Assert.assertTrue(outputSummary.exists(), "Expected output file " + outputSummary.getAbsolutePath() + " to exist.");
Assert.assertTrue(outputDetail.exists(), "Expected output file " + outputDetail.getAbsolutePath() + " to exist.");
Expand Down