Skip to content

Commit

Permalink
R Chart/PDF output filenames escape '%' with '%%' (#1671)
Browse files Browse the repository at this point in the history
* Allow for R Chart generated files to have '%' characters in their filenames
* Added specific test case  for this condition
  • Loading branch information
alanhoyle committed Apr 30, 2021
1 parent b675e57 commit 949d7f9
Show file tree
Hide file tree
Showing 11 changed files with 36 additions and 17 deletions.
2 changes: 1 addition & 1 deletion build_push_docker.sh
@@ -1,7 +1,7 @@
#!/usr/bin/env bash

# This script is used to build and deploy the GCR docker image for Picard
# The dockerhub iamge is built using a dockerhub automated build: https://hub.docker.com/r/broadinstitute/picard/builds
# The dockerhub image is built using a dockerhub automated build: https://hub.docker.com/r/broadinstitute/picard/builds

if [[ "$1" == "" ]]
then
Expand Down
Expand Up @@ -52,7 +52,7 @@
import java.util.Set;

/**
* A command line tool to read a BAM file and produce standard alignment metrics that would be applicable to any alignment.
* A command line tool to read a BAM file and produce standard alignment metrics that would be applicable to any alignment.
* Metrics to include, but not limited to:
* <ul>
* <li>Total number of reads (total, period, no exclusions)</li>
Expand All @@ -77,7 +77,7 @@
* <li>the paired end orientation is different that the expected orientation</li>
* <li>the read contains an SA tag (chimeric alignment)</li>
* </ul>
*
*
* @author Doug Voet (dvoet at broadinstitute dot org)
*/
@CommandLineProgramProperties(
Expand Down Expand Up @@ -192,7 +192,7 @@ protected void setup(final SAMFileHeader header, final File samFile) {

if (HISTOGRAM_FILE != null) {
final List<String> plotArgs = new ArrayList<>();
Collections.addAll(plotArgs, OUTPUT.getAbsolutePath(), HISTOGRAM_FILE.getAbsolutePath(), INPUT.getName());
Collections.addAll(plotArgs, OUTPUT.getAbsolutePath(), HISTOGRAM_FILE.getAbsolutePath().replaceAll("%", "%%"), INPUT.getName());

final int rResult = RExecutor.executeFromClasspath(HISTOGRAM_R_SCRIPT, plotArgs.toArray(new String[0]));
if (rResult != 0) {
Expand Down
Expand Up @@ -73,9 +73,9 @@ public class CollectBaseDistributionByCycle extends SinglePassSamProgram {
"of the Base Recalibration (BQSR) pre-processing step of the "+
"<a href='https://www.broadinstitute.org/gatk/guide/best-practices'>GATK Best Practices for Variant Discovery</a>, "+
"which aims to correct some types of systematic biases that affect the accuracy of base quality scores."+

"<p>Note: Metrics labeled as percentages are actually expressed as fractions!</p>"+

"<h4>Usage example:</h4>" +
"<pre>" +
"java -jar picard.jar CollectBaseDistributionByCycle \\<br />" +
Expand Down Expand Up @@ -140,7 +140,7 @@ protected void finish() {
} else {
final int rResult = RExecutor.executeFromClasspath("picard/analysis/baseDistributionByCycle.R",
OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
INPUT.getName(),
plotSubtitle);
if (rResult != 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/picard/analysis/CollectGcBiasMetrics.java
Expand Up @@ -211,7 +211,7 @@ private void writeResultsToFiles() {
RExecutor.executeFromClasspath(R_SCRIPT,
OUTPUT.getAbsolutePath(),
SUMMARY_OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
String.valueOf(SCAN_WINDOW_SIZE));
}
}
Expand Down
Expand Up @@ -172,7 +172,7 @@ protected String[] customCommandLineValidation() {
file.write(OUTPUT);

final List<String> plotArgs = new ArrayList<>();
Collections.addAll(plotArgs, OUTPUT.getAbsolutePath(), Histogram_FILE.getAbsolutePath(), INPUT.getName());
Collections.addAll(plotArgs, OUTPUT.getAbsolutePath(), Histogram_FILE.getAbsolutePath().replaceAll("%", "%%"), INPUT.getName());

if (HISTOGRAM_WIDTH != null) {
plotArgs.add(String.valueOf(HISTOGRAM_WIDTH));
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/picard/analysis/CollectRnaSeqMetrics.java
Expand Up @@ -202,7 +202,7 @@ protected void finish() {
if (CHART_OUTPUT != null && atLeastOneHistogram) {
final int rResult = RExecutor.executeFromClasspath("picard/analysis/rnaSeqCoverage.R",
OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
INPUT.getName(),
this.plotSubtitle);

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/picard/analysis/CollectRrbsMetrics.java
Expand Up @@ -186,7 +186,7 @@ protected int doWork() {
}
summaryFile.write(SUMMARY_OUT);
detailsFile.write(DETAILS_OUT);
RExecutor.executeFromClasspath(R_SCRIPT, DETAILS_OUT.getAbsolutePath(), SUMMARY_OUT.getAbsolutePath(), PLOTS_OUT.getAbsolutePath());
RExecutor.executeFromClasspath(R_SCRIPT, DETAILS_OUT.getAbsolutePath(), SUMMARY_OUT.getAbsolutePath(), PLOTS_OUT.getAbsolutePath().replaceAll("%", "%%"));
CloserUtil.close(samReader);
return 0;
}
Expand Down
Expand Up @@ -154,7 +154,7 @@ protected int doWork() {
} else {
final int rResult = RExecutor.executeFromClasspath("picard/analysis/wgsHistogram.R",
OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
INPUT.getName(),
plotSubtitle);
if (rResult != 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/picard/analysis/MeanQualityByCycle.java
Expand Up @@ -213,7 +213,7 @@ protected void finish() {
final int rResult = RExecutor.executeFromClasspath(
"picard/analysis/meanQualityByCycle.R",
OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
INPUT.getName(),
plotSubtitle);

Expand Down
Expand Up @@ -164,7 +164,7 @@ protected void finish() {
final int rResult = RExecutor.executeFromClasspath(
"picard/analysis/qualityScoreDistribution.R",
OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath(),
CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
INPUT.getName(),
this.plotSubtitle);

Expand Down
25 changes: 22 additions & 3 deletions src/test/java/picard/analysis/CollectInsertSizeMetricsTest.java
Expand Up @@ -264,6 +264,25 @@ public void testMultipleOrientationsForHistogram() throws IOException {
Assert.assertEquals(rResult, 0);
}

@Test
public void testPercentCharInPdfFilename() throws IOException {
final File input = new File(TEST_DATA_DIR, "insert_size_metrics_test.sam");
final File outfile = File.createTempFile("test_%_char", ".insert_size_metrics");
final File pdf = File.createTempFile("test_%_char", ".pdf");
pdf.deleteOnExit();

final String[] args = new String[] {
"INPUT=" + input.getAbsolutePath(),
"OUTPUT=" + outfile.getAbsolutePath(),
"Histogram_FILE=" + pdf.getAbsolutePath(),
"LEVEL=SAMPLE",
"LEVEL=LIBRARY",
"LEVEL=READ_GROUP"
};
Assert.assertEquals(runPicardCommandLine(args), 0);
Assert.assertTrue(pdf.exists());
}

@Test
public void testWidthOfMetrics() throws IOException {
final File testSamFile = File.createTempFile("CollectInsertSizeMetrics", ".bam");
Expand Down Expand Up @@ -321,11 +340,11 @@ public void testWidthOfMetrics() throws IOException {

final MetricsFile<InsertSizeMetrics, Comparable<?>> output = new MetricsFile<InsertSizeMetrics, Comparable<?>>();
output.read(new FileReader(outfile));

final List<InsertSizeMetrics> metrics = output.getMetrics();

Assert.assertEquals(metrics.size(), 1);

final InsertSizeMetrics metric = metrics.get(0);

Assert.assertEquals(metric.PAIR_ORIENTATION.name(), "FR");
Expand Down

0 comments on commit 949d7f9

Please sign in to comment.