Merge branch 'master' of ssh://nickel.broadinstitute.org/humgen/gsa-scr1/gsa-engineering/git/unstable

build.xml

@@ -48,8 +48,9 @@
     <property name="scaladoc.dir" value="scaladoc" />
     
     <!-- Contracts for Java -->
-    <!-- To disable, run with -Duse.contracts=false -->
+    <!-- By default, enabled only for test targets -->
-    <property name="use.contracts" value="false" />
+    <!-- To disable for test targets, run with -Duse.contracts=false -->
+    <!-- To enable for non-test targets, run with -Duse.contracts=true -->
     <property name="java.contracts" value="${build.dir}/java/contracts" />
     <property name="contracts.version" value="1.0-20110609" />
     <property name="cofoja.jar" value="${lib.dir}/cofoja-${contracts.version}.jar"/>
@@ -176,6 +177,10 @@
         <property name="scala.target" value="core"/>
     </target>
 
+    <target name="init.usecontracts">
+        <property name="use.contracts" value="true" />
+    </target>
+
     <target name="git.describe">
       <exec executable="git" outputproperty="git.describe.output" resultproperty="git.describe.exit.value" failonerror="false">
         <arg line="describe" />
@@ -485,11 +490,16 @@
             <pathelement location="${java.classes}" />
         </path>
 
+        <!-- Run with -Dgatkdocs.include.hidden=true to include documentation for hidden features -->
+        <condition property="gatkdocs.include.hidden.arg" value="-include-hidden" else="">
+            <isset property="gatkdocs.include.hidden" />
+        </condition>
+
         <javadoc doclet="org.broadinstitute.sting.utils.help.GATKDoclet"
                  docletpathref="doclet.classpath"
                  classpathref="external.dependencies"
                  classpath="${java.classes}"
-                 additionalparam="-private -build-timestamp &quot;${build.timestamp}&quot; -absolute-version ${build.version} -quiet -J-Xdebug -J-Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=5005"> <!-- -test to only do DocumentationTest walker -->
+                 additionalparam="${gatkdocs.include.hidden.arg} -private -build-timestamp &quot;${build.timestamp}&quot; -absolute-version ${build.version} -quiet -J-Xdebug -J-Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=5005"> <!-- -test to only do DocumentationTest walker -->
             <sourcefiles>
                 <union>
                     <fileset refid="all.java.source.files"/>
@@ -744,8 +754,7 @@
         </scalac>
     </target>
 
-    <target name="test.compile" depends="test.java.compile,test.scala.compile">
+    <target name="test.compile" depends="init.usecontracts,test.java.compile,test.scala.compile" />
-    </target>
 
     <!-- new scala target -->
     

public/java/src/org/broadinstitute/sting/analyzecovariates/AnalyzeCovariates.java

@@ -31,6 +31,7 @@ import org.broadinstitute.sting.commandline.Input;
 import org.broadinstitute.sting.gatk.walkers.recalibration.Covariate;
 import org.broadinstitute.sting.gatk.walkers.recalibration.RecalDatum;
 import org.broadinstitute.sting.gatk.walkers.recalibration.RecalibrationArgumentCollection;
+import org.broadinstitute.sting.utils.R.RScriptExecutor;
 import org.broadinstitute.sting.utils.classloader.PluginManager;
 import org.broadinstitute.sting.utils.exceptions.DynamicClassResolutionException;
 import org.broadinstitute.sting.utils.help.DocumentedGATKFeature;
@@ -38,6 +39,7 @@ import org.broadinstitute.sting.utils.text.XReadLines;
 
 import java.io.*;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Map;
 import java.util.regex.Pattern;
@@ -323,13 +325,14 @@ public class AnalyzeCovariates extends CommandLineProgram {
     }
 
     private void callRScripts() {
+        RScriptExecutor.RScriptArgumentCollection argumentCollection =
+                new RScriptExecutor.RScriptArgumentCollection(PATH_TO_RSCRIPT, Arrays.asList(PATH_TO_RESOURCES));
+        RScriptExecutor executor = new RScriptExecutor(argumentCollection, true);
 
         int numReadGroups = 0;
 
         // for each read group
         for( Object readGroupKey : dataManager.getCollapsedTable(0).data.keySet() ) {
-
-            Process p;
             if(++numReadGroups <= NUM_READ_GROUPS_TO_PROCESS || NUM_READ_GROUPS_TO_PROCESS == -1) {
 
                 String readGroup = readGroupKey.toString();
@@ -338,36 +341,20 @@ public class AnalyzeCovariates extends CommandLineProgram {
                 // for each covariate
                 for( int iii = 1; iii < requestedCovariates.size(); iii++ ) {
                     Covariate cov = requestedCovariates.get(iii);
-                    try {
+                    final String outputFilename = OUTPUT_DIR + readGroup + "." + cov.getClass().getSimpleName()+ ".dat";
-
                     if (DO_INDEL_QUALITY) {
-                            p = Runtime.getRuntime().exec(PATH_TO_RSCRIPT + " " + PATH_TO_RESOURCES + "plot_indelQuality.R" + " " +
+                        executor.callRScripts("plot_indelQuality.R", outputFilename,
-                                         OUTPUT_DIR + readGroup + "." + cov.getClass().getSimpleName()+ ".dat" + " " +
                                 cov.getClass().getSimpleName().split("Covariate")[0]); // The third argument is the name of the covariate in order to make the plots look nice
-                             p.waitFor();
-                            
                     }   else {
                         if( iii == 1 ) {
                             // Analyze reported quality
-                                p = Runtime.getRuntime().exec(PATH_TO_RSCRIPT + " " + PATH_TO_RESOURCES + "plot_residualError_QualityScoreCovariate.R" + " " +
+                            executor.callRScripts("plot_residualError_QualityScoreCovariate.R", outputFilename,
-                                            OUTPUT_DIR + readGroup + "." + cov.getClass().getSimpleName()+ ".dat" + " " +
+                                    IGNORE_QSCORES_LESS_THAN, MAX_QUALITY_SCORE, MAX_HISTOGRAM_VALUE); // The third argument is the Q scores that should be turned pink in the plot because they were ignored
-                                            IGNORE_QSCORES_LESS_THAN + " " + MAX_QUALITY_SCORE + " " + MAX_HISTOGRAM_VALUE); // The third argument is the Q scores that should be turned pink in the plot because they were ignored
-                                p.waitFor();
                         } else { // Analyze all other covariates
-                                p = Runtime.getRuntime().exec(PATH_TO_RSCRIPT + " " + PATH_TO_RESOURCES + "plot_residualError_OtherCovariate.R" + " " +
+                            executor.callRScripts("plot_residualError_OtherCovariate.R", outputFilename,
-                                            OUTPUT_DIR + readGroup + "." + cov.getClass().getSimpleName()+ ".dat" + " " +
                                     cov.getClass().getSimpleName().split("Covariate")[0]); // The third argument is the name of the covariate in order to make the plots look nice
-                                p.waitFor();
                         }
                     }
-                    } catch (InterruptedException e) {
-                        e.printStackTrace();
-                        System.exit(-1);
-                    } catch (IOException e) {
-                        System.out.println("Fatal Exception: Perhaps RScript jobs are being spawned too quickly? One work around is to process fewer read groups using the -numRG option.");
-                        e.printStackTrace();
-                        System.exit(-1);
-                    }
                 }
             } else { // at the maximum number of read groups so break out
                 break;

public/java/src/org/broadinstitute/sting/gatk/report/GATKReportTable.java

@@ -92,6 +92,8 @@ import java.util.regex.Pattern;
  * @author Khalid Shakir
  */
 public class GATKReportTable {
+    /** REGEX that matches any table with an invalid name */
+    public final static String INVALID_TABLE_NAME_REGEX = "[^a-zA-Z0-9_\\-\\.]";
     private static final GATKReportVersion LATEST_REPORT_VERSION = GATKReportVersion.V0_2;
     private String tableName;
     private String tableDescription;
@@ -111,7 +113,7 @@ public class GATKReportTable {
      * @return  true if the name is valid, false if otherwise
      */
     private boolean isValidName(String name) {
-        Pattern p = Pattern.compile("[^a-zA-Z0-9_\\-\\.]");
+        Pattern p = Pattern.compile(INVALID_TABLE_NAME_REGEX);
         Matcher m = p.matcher(name);
 
         return !m.find();

public/java/src/org/broadinstitute/sting/utils/R/RScriptExecutor.java

@@ -32,6 +32,7 @@ import org.broadinstitute.sting.commandline.ArgumentCollection;
 import org.broadinstitute.sting.gatk.walkers.recalibration.Covariate;
 import org.broadinstitute.sting.utils.PathUtils;
 import org.broadinstitute.sting.utils.Utils;
+import org.broadinstitute.sting.utils.exceptions.UserException;
 
 import java.io.File;
 import java.io.IOException;
@@ -53,11 +54,19 @@ public class RScriptExecutor {
     public static class RScriptArgumentCollection {
         @Advanced
         @Argument(fullName = "path_to_Rscript", shortName = "Rscript", doc = "The path to your implementation of Rscript. For Broad users this is maybe /broad/tools/apps/R-2.6.0/bin/Rscript", required = false)
-        private String PATH_TO_RSCRIPT = "Rscript";
+        public String PATH_TO_RSCRIPT = "Rscript";
 
         @Advanced
-        @Argument(fullName = "path_to_resources", shortName = "resources", doc = "Path to resources folder holding the Sting R scripts.", required = false)
+        @Argument(fullName = "path_to_Rresources", shortName = "Rresources", doc = "Path to resources folder holding the Sting R scripts.", required = false)
-        private List<String> PATH_TO_RESOURCES = Arrays.asList("public/R/", "private/R/");
+        public List<String> PATH_TO_RESOURCES = Arrays.asList("public/R/", "private/R/");
+
+        public RScriptArgumentCollection() {}
+
+        /** For testing and convenience */
+        public RScriptArgumentCollection(final String PATH_TO_RSCRIPT, final List<String> PATH_TO_RESOURCES) {
+            this.PATH_TO_RSCRIPT = PATH_TO_RSCRIPT;
+            this.PATH_TO_RESOURCES = PATH_TO_RESOURCES;
+        }
     }
 
     final RScriptArgumentCollection myArgs;
@@ -68,11 +77,11 @@ public class RScriptExecutor {
         this.exceptOnError = exceptOnError;
     }
 
-    public void callRScripts(String scriptName, String... scriptArgs) {
+    public void callRScripts(String scriptName, Object... scriptArgs) {
         callRScripts(scriptName, Arrays.asList(scriptArgs));
     }
 
-    public void callRScripts(String scriptName, List<String> scriptArgs) {
+    public void callRScripts(String scriptName, List<Object> scriptArgs) {
         try {
             final File pathToScript = findScript(scriptName);
             if ( pathToScript == null ) return; // we failed but shouldn't exception out
@@ -98,7 +107,7 @@ public class RScriptExecutor {
             }
         }
 
-        generateException("Couldn't find script: " + scriptName + " in " + myArgs.PATH_TO_RSCRIPT);
+        generateException("Couldn't find script: " + scriptName + " in " + myArgs.PATH_TO_RESOURCES);
         return null;
     }
 
@@ -112,7 +121,7 @@ public class RScriptExecutor {
 
     private void generateException(String msg, Throwable e) {
         if ( exceptOnError )
-            throw new RuntimeException(msg, e);
+            throw new UserException(msg, e);
         else
             logger.warn(msg + (e == null ? "" : ":" + e.getMessage()));
     }

public/java/test/org/broadinstitute/sting/utils/R/RScriptExecutorUnitTest.java

@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2011, The Broad Institute
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+package org.broadinstitute.sting.utils.R;
+
+import org.apache.commons.io.FileUtils;
+import org.broadinstitute.sting.BaseTest;
+import org.broadinstitute.sting.gatk.walkers.diffengine.DiffElement;
+import org.broadinstitute.sting.gatk.walkers.diffengine.DiffEngine;
+import org.broadinstitute.sting.gatk.walkers.diffengine.DiffNode;
+import org.broadinstitute.sting.gatk.walkers.diffengine.Difference;
+import org.broadinstitute.sting.utils.exceptions.UserException;
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Basic unit test for RScriptExecutor in reduced reads
+ */
+public class RScriptExecutorUnitTest extends BaseTest {
+    final static String testrscript = "print(\"hello, world\")\n";
+    final static String publicRScript = "plot_Tranches.R";
+    final static String privateRScript = "variantCallQC.R";
+
+    // --------------------------------------------------------------------------------
+    //
+    // Difference testing routines
+    //
+    // --------------------------------------------------------------------------------
+
+    private void testOne(String script, String pathToRscript, String anotherSearchPath, boolean exceptOnError) {
+        RScriptExecutor.RScriptArgumentCollection collection =
+                new RScriptExecutor.RScriptArgumentCollection();
+        if ( pathToRscript != null )
+            collection.PATH_TO_RSCRIPT = pathToRscript;
+        if ( anotherSearchPath != null ) {
+            List<String> x = new ArrayList<String>(collection.PATH_TO_RESOURCES);
+            x.add(anotherSearchPath);
+            collection.PATH_TO_RESOURCES = x;
+        }
+        RScriptExecutor executor = new RScriptExecutor(collection, exceptOnError);
+        executor.callRScripts(script);
+    }
+
+    @Test
+    public void testPublic() { testOne(publicRScript, null, null, true); }
+
+    @Test
+    public void testPrivate() { testOne(privateRScript, null, null, true); }
+
+    // make sure we don't break finding something in private by adding another directory
+    @Test
+    public void testPrivateWithAdditionalPath1() { testOne(privateRScript, null, "dist", true); }
+
+    // make sure we don't break finding something in private by adding another directory
+    @Test
+    public void testPrivateWithAdditionalPath2() { testOne(privateRScript, null, "doesNotExist", true); }
+
+    @Test(expectedExceptions = UserException.class)
+    public void testNonExistantScriptException() { testOne("does_not_exist.R", null, null, true); }
+
+    @Test()
+    public void testNonExistantScriptNoException() { testOne("does_not_exist.R", null, null, false); }
+
+    @Test(expectedExceptions = UserException.class)
+    public void testNonExistantRScriptException() { testOne(publicRScript, "badRScriptValue", null, true); }
+
+    @Test()
+    public void testNonExistantRScriptNoException() { testOne(publicRScript, "badRScriptValue", null, false); }
+
+    @Test()
+    public void testScriptInNewPath() throws IOException {
+        File t = createTempFile("myTestScript", ".R");
+        FileUtils.writeStringToFile(t, testrscript);
+        testOne(t.getName(), null, t.getParent(), true);
+    }
+}

public/scala/src/org/broadinstitute/sting/queue/util/QJobReport.scala

@@ -51,7 +51,7 @@ trait QJobReport extends Logging {
     logger.info("info " + info)
     reportFeatures = Map(
       "iteration" -> 1,
-      "analysisName" -> self.analysisName,
+      "analysisName" -> getReportGroup,
       "jobName" -> QJobReport.workAroundSameJobNames(this),
       "intermediate" -> self.isIntermediate,
       "startTime" -> info.getStartTime.getTime,
@@ -59,15 +59,12 @@ trait QJobReport extends Logging {
       "formattedStartTime" -> info.getFormattedStartTime,
       "formattedDoneTime" -> info.getFormattedDoneTime,
       "runtime" -> info.getRuntimeInMs).mapValues((x:Any) => if (x != null) x.toString else "null") ++ reportFeatures
-
+    // note -- by adding reportFeatures second we override iteration
-//    // handle the special case of iterations
+    // (or any other binding) with the user provided value
-//    reportFeatures.get("iteration") match {
-//      case None => reportFeatures("iteration") = 1
-//      case _ => ;
-//    }
   }
 
-  def getReportGroup = analysisName
+  /** The report Group is the analysis name transform to only contain valid GATKReportTable characters */
+  def getReportGroup = self.analysisName.replaceAll(GATKReportTable.INVALID_TABLE_NAME_REGEX, "_")
   def getReportFeatures = reportFeatures
 
   def getReportFeatureNames: List[String] = getReportFeatures.keys.toList