Obey the tmpDir setting in several constructors that currently ignore it #826

Merged
merged 2 commits into from May 2, 2017
Jump to file or symbol
Failed to load files and symbols.
+54 −16
Split
@@ -174,6 +174,14 @@ public SAMFileWriterFactory setMaxRecordsInRam(final int maxRecordsInRam) {
}
/**
+ * Gets the maximum number of records held in RAM before spilling to disk during sorting.
+ * @see #setMaxRecordsInRam(int)
+ */
+ public int getMaxRecordsInRam() {
+ return maxRecordsInRam;
+ }
+
+ /**
* Turn on or off the use of asynchronous IO for writing output SAM and BAM files. If true then
* each SAMFileWriter creates a dedicated thread which is used for compression and IO activities.
*/
@@ -211,6 +219,14 @@ public SAMFileWriterFactory setTempDirectory(final File tmpDir) {
}
/**
+ * Gets the temporary directory that will be used when sorting data.
+ * @see #setTempDirectory(File)
+ */
+ public File getTempDirectory() {
+ return tmpDir;
+ }
+
+ /**
* Set the flag output format only when writing text.
* Default value: [[htsjdk.samtools.SAMTextWriter.samFlagFieldOutput.DECIMAL]]
*/
@@ -253,7 +269,6 @@ public SAMFileWriter makeBAMWriter(final SAMFileHeader header, final boolean pre
if (this.createIndex && !createIndex) {
log.warn("Cannot create index for BAM because output file is not a regular file: " + outputFile.getAbsolutePath());
}
- if (this.tmpDir != null) ret.setTempDirectory(this.tmpDir);
initializeBAMWriter(ret, header, presorted, createIndex);
if (this.useAsyncIo) return new AsyncSAMFileWriter(ret, this.asyncOutputBufferSize);
@@ -268,6 +283,7 @@ private void initializeBAMWriter(final BAMFileWriter writer, final SAMFileHeader
if (maxRecordsInRam != null) {
writer.setMaxRecordsInRam(maxRecordsInRam);
}
+ if (this.tmpDir != null) writer.setTempDirectory(this.tmpDir);
writer.setHeader(header);
if (createIndex && writer.getSortOrder().equals(SAMFileHeader.SortOrder.coordinate)) {
writer.enableBamIndexConstruction();
@@ -294,14 +310,7 @@ public SAMFileWriter makeSAMWriter(final SAMFileHeader header, final boolean pre
? new SAMTextWriter(new Md5CalculatingOutputStream(new FileOutputStream(outputFile, false),
new File(outputFile.getAbsolutePath() + ".md5")), samFlagFieldOutput)
: new SAMTextWriter(outputFile, samFlagFieldOutput);
- ret.setSortOrder(header.getSortOrder(), presorted);
- if (maxRecordsInRam != null) {
- ret.setMaxRecordsInRam(maxRecordsInRam);
- }
- ret.setHeader(header);
-
- if (this.useAsyncIo) return new AsyncSAMFileWriter(ret, this.asyncOutputBufferSize);
- else return ret;
+ return initWriter(header, presorted, ret);
} catch (final IOException ioe) {
throw new RuntimeIOException("Error opening file: " + outputFile.getAbsolutePath());
}
@@ -324,7 +333,7 @@ public SAMFileWriter makeSAMWriter(final SAMFileHeader header, final boolean pre
if (samFlagFieldOutput == SamFlagField.NONE) {
samFlagFieldOutput = Defaults.SAM_FLAG_FIELD_FORMAT;
}
- return initWriter(header, presorted, false, new SAMTextWriter(stream, samFlagFieldOutput));
+ return initWriter(header, presorted, new SAMTextWriter(stream, samFlagFieldOutput));
}
/**
@@ -338,24 +347,23 @@ public SAMFileWriter makeSAMWriter(final SAMFileHeader header, final boolean pre
*/
public SAMFileWriter makeBAMWriter(final SAMFileHeader header, final boolean presorted, final OutputStream stream) {
- return initWriter(header, presorted, true, new BAMFileWriter(stream, null, this.getCompressionLevel(), this.deflaterFactory));
+ return initWriter(header, presorted, new BAMFileWriter(stream, null, this.getCompressionLevel(), this.deflaterFactory));
}
/**
* Initialize SAMTextWriter or a BAMFileWriter and possibly wrap in AsyncSAMFileWriter
- *
* @param header entire header. Sort order is determined by the sortOrder property of this arg.
* @param presorted if true, SAMRecords must be added to the SAMFileWriter in order that agrees with header.sortOrder.
- * @param binary do we want to generate a BAM or a SAM
* @param writer SAM or BAM writer to initialize and maybe wrap.
*/
- private SAMFileWriter initWriter(final SAMFileHeader header, final boolean presorted, final boolean binary,
+ private SAMFileWriter initWriter(final SAMFileHeader header, final boolean presorted,
final SAMFileWriterImpl writer) {
writer.setSortOrder(header.getSortOrder(), presorted);
if (maxRecordsInRam != null) {
writer.setMaxRecordsInRam(maxRecordsInRam);
}
+ if (this.tmpDir != null) writer.setTempDirectory(this.tmpDir);
writer.setHeader(header);
if (this.useAsyncIo) return new AsyncSAMFileWriter(writer, this.asyncOutputBufferSize);
@@ -111,7 +111,11 @@ void setMaxRecordsInRam(final int maxRecordsInRam) {
}
this.maxRecordsInRam = maxRecordsInRam;
}
-
+
+ int getMaxRecordsInRam() {
+ return maxRecordsInRam;
+ }
+
/**
* When writing records that are not presorted, specify the path of the temporary directory
* for spilling to disk. Must be called before setHeader().
@@ -123,6 +127,10 @@ void setTempDirectory(final File tmpDir) {
}
}
+ File getTempDirectory() {
+ return tmpDir;
+ }
+
/**
* Must be called before addAlignment. Header cannot be null.
*/
@@ -167,7 +167,29 @@ private void createSmallBamToOutputStream(final OutputStream outputStream,boolea
fillSmallBam(writer);
writer.close();
}
-
+
+ @Test(description="check that factory settings are propagated to writer")
+ public void testFactorySettings() throws Exception {
+ final SAMFileWriterFactory factory = new SAMFileWriterFactory();
+ factory.setCreateIndex(false);
+ factory.setCreateMd5File(false);
+ final File wontBeUsed = new File("wontBeUsed.tmp");
+ final int maxRecsInRam = 271828;
+ factory.setMaxRecordsInRam(maxRecsInRam);
+ factory.setTempDirectory(wontBeUsed);
+ final SAMFileHeader header = new SAMFileHeader();
+ header.setSortOrder(SAMFileHeader.SortOrder.coordinate);
+ header.addSequence(new SAMSequenceRecord("chr1", 123));
+ try (final SAMFileWriter writer = factory.makeBAMWriter(header, false, new ByteArrayOutputStream())) {
+ Assert.assertEquals(maxRecsInRam, ((SAMFileWriterImpl) writer).getMaxRecordsInRam());
+ Assert.assertEquals(wontBeUsed, ((SAMFileWriterImpl) writer).getTempDirectory());
+ }
+ try (final SAMFileWriter writer = factory.makeSAMWriter(header, false, new ByteArrayOutputStream())) {
@yfarjoun

yfarjoun Apr 18, 2017

Contributor

is this code duplicated on purpose?

@Lenbok

Lenbok Apr 18, 2017

Contributor

Notice that one is testing SAM, and one is testing BAM. It didn't seem worth pulling out the two duplicated assertions to a separate method.

@yfarjoun

yfarjoun Apr 18, 2017

Contributor

ah. OK. sorry. I didn't see that.

+ Assert.assertEquals(maxRecsInRam, ((SAMFileWriterImpl) writer).getMaxRecordsInRam());
+ Assert.assertEquals(wontBeUsed, ((SAMFileWriterImpl) writer).getTempDirectory());
+ }
+ }
+
private int fillSmallBam(SAMFileWriter writer) {
final SAMRecordSetBuilder builder = new SAMRecordSetBuilder();
builder.addUnmappedFragment("HiMom!");