From cec7107762937832f4014f3df0830a45da4757d9 Mon Sep 17 00:00:00 2001
From: Mauricio Carneiro <carneiro@broadinstitute.org>
Date: Sat, 14 Jan 2012 12:13:15 -0500
Subject: [PATCH] Better location for the downsampling of reads in PrintReads

   * using the filter() instead of map() makes for a cleaner walker.
   * renaming the unit tests to make more sense with the other unit and integration tests
---
 .../sting/gatk/walkers/PrintReadsWalker.java  | 19 +++++++++----------
 ...rUnitTest.java => PrintReadsUnitTest.java} | 15 +++++++--------
 2 files changed, 16 insertions(+), 18 deletions(-)
 rename public/java/test/org/broadinstitute/sting/gatk/walkers/{PrintReadsWalkerUnitTest.java => PrintReadsUnitTest.java} (97%)

diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/PrintReadsWalker.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/PrintReadsWalker.java
index f029b768e..0702b08c1 100755
--- a/public/java/src/org/broadinstitute/sting/gatk/walkers/PrintReadsWalker.java
+++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/PrintReadsWalker.java
@@ -31,8 +31,11 @@ import net.sf.samtools.SAMRecord;
 import org.broadinstitute.sting.commandline.Argument;
 import org.broadinstitute.sting.commandline.Output;
 import org.broadinstitute.sting.gatk.GenomeAnalysisEngine;
+import org.broadinstitute.sting.gatk.contexts.ReferenceContext;
+import org.broadinstitute.sting.gatk.refdata.ReadMetaDataTracker;
 import org.broadinstitute.sting.utils.SampleUtils;
 import org.broadinstitute.sting.utils.baq.BAQ;
+import org.broadinstitute.sting.utils.sam.GATKSAMRecord;
 
 import java.io.File;
 import java.util.Collection;
@@ -40,10 +43,6 @@ import java.util.Random;
 import java.util.Set;
 import java.util.TreeSet;
 
-import org.broadinstitute.sting.gatk.contexts.ReferenceContext;
-import org.broadinstitute.sting.gatk.refdata.ReadMetaDataTracker;
-import org.broadinstitute.sting.utils.sam.GATKSAMRecord;
-
 /**
  * Renders, in SAM/BAM format, all reads from the input data set in the order in which they appear in the input file.
  *
@@ -201,18 +200,19 @@ public class PrintReadsWalker extends ReadWalker<SAMRecord, SAMFileWriter> {
             nReadsToPrint--;       // n > 0 means there are still reads to be printed.
         }
 
-        return true;
-	}
+        // if downsample option is turned off (= 1) then don't waste time  getting the next random number.
+        return (downsampleRatio == 1 || random.nextDouble() < downsampleRatio);
+    }
 
     /**
      * The reads map function.
      *
-     * @param ref the reference bases that correspond to our read, if a reference was provided
+     * @param ref  the reference bases that correspond to our read, if a reference was provided
      * @param read the read itself, as a SAMRecord
      * @return the read itself
      */
     public SAMRecord map( ReferenceContext ref, GATKSAMRecord read, ReadMetaDataTracker metaDataTracker ) {
-        return (random.nextDouble() < downsampleRatio) ? read : null;
+        return read;
     }
 
     /**
@@ -233,8 +233,7 @@ public class PrintReadsWalker extends ReadWalker<SAMRecord, SAMFileWriter> {
      * @return the SAMFileWriter, so that the next reduce can emit to the same source
      */
     public SAMFileWriter reduce( SAMRecord read, SAMFileWriter output ) {
-        if (read != null)
-            output.addAlignment(read);
+        output.addAlignment(read);
         return output;
     }
 
diff --git a/public/java/test/org/broadinstitute/sting/gatk/walkers/PrintReadsWalkerUnitTest.java b/public/java/test/org/broadinstitute/sting/gatk/walkers/PrintReadsUnitTest.java
similarity index 97%
rename from public/java/test/org/broadinstitute/sting/gatk/walkers/PrintReadsWalkerUnitTest.java
rename to public/java/test/org/broadinstitute/sting/gatk/walkers/PrintReadsUnitTest.java
index 484641981..0fcaad3bf 100644
--- a/public/java/test/org/broadinstitute/sting/gatk/walkers/PrintReadsWalkerUnitTest.java
+++ b/public/java/test/org/broadinstitute/sting/gatk/walkers/PrintReadsUnitTest.java
@@ -1,20 +1,19 @@
 package org.broadinstitute.sting.gatk.walkers;
 
+import net.sf.samtools.SAMFileHeader;
+import net.sf.samtools.SAMRecord;
 import org.broadinstitute.sting.BaseTest;
 import org.broadinstitute.sting.gatk.contexts.ReferenceContext;
 import org.broadinstitute.sting.utils.sam.ArtificialReadsTraversal;
 import org.broadinstitute.sting.utils.sam.ArtificialSAMFileWriter;
 import org.broadinstitute.sting.utils.sam.ArtificialSAMUtils;
-import net.sf.samtools.SAMRecord;
-import net.sf.samtools.SAMFileHeader;
-
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertTrue;
-
 import org.broadinstitute.sting.utils.sam.GATKSAMRecord;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
 
 /*
  * Copyright (c) 2009 The Broad Institute
@@ -44,11 +43,11 @@ import org.testng.annotations.Test;
 /**
  * @author aaron
  *         <p/>
- *         Class PrintReadsWalkerUnitTest
+ *         Class PrintReadsUnitTest
  *         <p/>
  *         This tests the print reads walker, using the artificial reads traversal
  */
-public class PrintReadsWalkerUnitTest extends BaseTest {
+public class PrintReadsUnitTest extends BaseTest {
 
     /**
      * our private fake reads traversal.  This traversal seeds the