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

Changes from 1 commit
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
15 changes: 6 additions & 9 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 @@ -224,7 +224,9 @@ protected int doWork() {
outputDetailMetricsFile = DETAIL_OUTPUT;
outputSummaryMetricsFile = SUMMARY_OUTPUT;
} else {
if (!OUTPUT.endsWith(".")) OUTPUT = OUTPUT + ".";
if (!OUTPUT.endsWith(".")) {
OUTPUT = OUTPUT + ".";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
OUTPUT = OUTPUT + ".";
OUTPUT += ".";

}
outputDetailMetricsFile = new File(OUTPUT + FINGERPRINT_DETAIL_FILE_SUFFIX);
outputSummaryMetricsFile = new File(OUTPUT + FINGERPRINT_SUMMARY_FILE_SUFFIX);
}
Expand Down Expand Up @@ -366,16 +368,15 @@ 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) {
anyNonzeroLod |= true;
if (metrics.LOD_EXPECTED_SAMPLE != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

you could write

anyNonzeroLod |= metrics.LOD_EXPECTED_SAMPLE != 0;

(also, I think the proper spelling is anyNonZeroLod as non-zero is a hyphenated word)

Conversely avoiding negatives would make this a bit nicer, IMO, e.g.

allZeroLod &= metrics.LOD_EXPECTED_SAMPLE == 0;
...
if (allZeroLod) {

I find reading code with double negatives like !anyNonZeroLod to be unnecessarily confusing. The same is true of the text No Non-zero results found.

anyNonzeroLod = true;
}
}

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


if (!anyNonzeroLod){
if (!anyNonzeroLod) {
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.");
Expand All @@ -401,10 +402,6 @@ protected String[] customCommandLineValidation() {
return super.customCommandLineValidation();
}

static boolean isBamOrSam(final File f) {
return isBamOrSam(f.toPath());
}

static boolean isBamOrSam(final Path p) {
return (p.toUri().getRawPath().endsWith(BamFileIoUtils.BAM_FILE_EXTENSION) || p.toUri().getRawPath().endsWith(IOUtil.SAM_FILE_EXTENSION));
}
Expand Down