diff --git a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/CopyNumberArgumentValidationUtils.java b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/CopyNumberArgumentValidationUtils.java index 42740f8ecb5..d4c411562fe 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/CopyNumberArgumentValidationUtils.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/CopyNumberArgumentValidationUtils.java @@ -5,10 +5,7 @@ import htsjdk.samtools.SAMSequenceRecord; import htsjdk.samtools.util.Locatable; import org.broadinstitute.hellbender.cmdline.argumentcollections.IntervalArgumentCollection; -import org.broadinstitute.hellbender.utils.IntervalMergingRule; -import org.broadinstitute.hellbender.utils.IntervalSetRule; -import org.broadinstitute.hellbender.utils.IntervalUtils; -import org.broadinstitute.hellbender.utils.Utils; +import org.broadinstitute.hellbender.utils.*; import java.util.Iterator; import java.util.List; @@ -36,12 +33,15 @@ public static void validateIntervalArgumentCollection(final IntervalArgumentColl } /** - * Validate that a list of locatables is sorted according to a sequence dictionary and contains no duplicates or overlaps. + * Validate that a list of locatables is valid and sorted according to a sequence dictionary and contains no duplicates or overlaps. */ public static void validateIntervals(final List intervals, final SAMSequenceDictionary sequenceDictionary) { Utils.nonNull(intervals); Utils.nonNull(sequenceDictionary); + final GenomeLocParser genomeLocParser = new GenomeLocParser(sequenceDictionary); + Utils.validateArg(intervals.stream().allMatch(i -> genomeLocParser.isValidGenomeLoc(i.getContig(), i.getStart(), i.getEnd())), + "Records contained at least one interval that did not validate against the sequence dictionary."); if (!Ordering.from(IntervalUtils.getDictionaryOrderComparator(sequenceDictionary)).isStrictlyOrdered(intervals)) { throw new IllegalArgumentException("Records were not strictly sorted in dictionary order."); } diff --git a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractLocatableCollection.java b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractLocatableCollection.java index f0860c411f2..935fb2b1c97 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractLocatableCollection.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractLocatableCollection.java @@ -36,12 +36,7 @@ public abstract class AbstractLocatableCollection recordFromDataLineDecoder, final BiConsumer recordToDataLineEncoder) { - super( - metadata, - sortRecords(records, metadata.getSequenceDictionary()), - mandatoryColumns, - recordFromDataLineDecoder, - recordToDataLineEncoder); + super(metadata, sortRecords(records, metadata.getSequenceDictionary()), mandatoryColumns, recordFromDataLineDecoder, recordToDataLineEncoder); CopyNumberArgumentValidationUtils.validateIntervals(getRecords(), metadata.getSequenceDictionary()); } diff --git a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractRecordCollection.java b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractRecordCollection.java index 7b876372d58..c1ada61e681 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractRecordCollection.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractRecordCollection.java @@ -19,7 +19,7 @@ import java.util.stream.Collectors; /** - * Represents {@link METADATA} (which can be represented as a {@link SAMFileHeader}, + * Represents {@link METADATA} (which can be represented as a {@link SAMFileHeader}), * an immutable collection of records, * a set of mandatory column headers given by a {@link TableColumnCollection}, * and lambdas for reading and writing records. @@ -28,7 +28,6 @@ */ public abstract class AbstractRecordCollection { private final METADATA metadata; - private final SAMFileHeader header; private final ImmutableList records; private final TableColumnCollection mandatoryColumns; private final Function recordFromDataLineDecoder; @@ -50,7 +49,6 @@ public abstract class AbstractRecordCollection recordFromDataLineDecoder, final BiConsumer recordToDataLineEncoder) { this.metadata = Utils.nonNull(metadata); - header = metadata.toHeader(); this.records = ImmutableList.copyOf(Utils.nonNull(records)); this.mandatoryColumns = Utils.nonNull(mandatoryColumns); this.recordFromDataLineDecoder = Utils.nonNull(recordFromDataLineDecoder); @@ -78,8 +76,7 @@ public abstract class AbstractRecordCollection getRecords() { * Writes the records to file. */ public void write(final File outputFile) { - try (final FileWriter writer = new FileWriter(outputFile, true)) { - writer.write(header.getSAMString()); - try (final RecordWriter recordWriter = new RecordWriter(writer)) { - recordWriter.writeAllRecords(records); - } + try (final FileWriter writer = new FileWriter(outputFile)) { + writer.write(metadata.toHeader().getSAMString()); + } catch (final IOException e) { + throw new UserException.CouldNotCreateOutputFile(outputFile, e); + } + try (final RecordWriter recordWriter = new RecordWriter(new FileWriter(outputFile, true))) { + recordWriter.writeAllRecords(records); } catch (final IOException e) { throw new UserException.CouldNotCreateOutputFile(outputFile, e); } @@ -142,7 +141,7 @@ public boolean equals(Object o) { @Override public int hashCode() { - int result = header.hashCode(); + int result = metadata.hashCode(); result = 31 * result + records.hashCode(); result = 31 * result + mandatoryColumns.hashCode(); result = 31 * result + recordFromDataLineDecoder.hashCode(); diff --git a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleLocatableCollection.java b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleLocatableCollection.java index cb7628bb1a1..f08fa15220e 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleLocatableCollection.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleLocatableCollection.java @@ -31,12 +31,7 @@ public abstract class AbstractSampleLocatableCollection recordFromDataLineDecoder, final BiConsumer recordToDataLineEncoder) { - super( - metadata, - records, - mandatoryColumns, - recordFromDataLineDecoder, - recordToDataLineEncoder); + super(metadata, records, mandatoryColumns, recordFromDataLineDecoder, recordToDataLineEncoder); } /** diff --git a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleRecordCollection.java b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleRecordCollection.java index a1eff6bb875..b34cce75e9b 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleRecordCollection.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleRecordCollection.java @@ -24,12 +24,7 @@ public abstract class AbstractSampleRecordCollection extends AbstractRec final TableColumnCollection mandatoryColumns, final Function recordFromDataLineDecoder, final BiConsumer recordToDataLineEncoder) { - super( - metadata, - records, - mandatoryColumns, - recordFromDataLineDecoder, - recordToDataLineEncoder); + super(metadata, records, mandatoryColumns, recordFromDataLineDecoder, recordToDataLineEncoder); } AbstractSampleRecordCollection(final File inputFile, diff --git a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleLocatableMetadata.java b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleLocatableMetadata.java index 362e4d514dc..f7c8fe7f62b 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleLocatableMetadata.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleLocatableMetadata.java @@ -5,6 +5,8 @@ import org.broadinstitute.hellbender.utils.Utils; /** + * Metadata associated with a collection of locatables. + * * @author Samuel Lee <slee@broadinstitute.org> */ public class SimpleLocatableMetadata implements LocatableMetadata { diff --git a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleSampleLocatableMetadata.java b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleSampleLocatableMetadata.java index 6bbca870dee..bc64ce95ff4 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleSampleLocatableMetadata.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleSampleLocatableMetadata.java @@ -5,6 +5,8 @@ import org.broadinstitute.hellbender.utils.Utils; /** + * Metadata associated with a collection of locatables associated with a single sample. + * * @author Samuel Lee <slee@broadinstitute.org> */ public class SimpleSampleLocatableMetadata implements SampleLocatableMetadata { diff --git a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleSampleMetadata.java b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleSampleMetadata.java index fc63cf84a0a..831748b537a 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleSampleMetadata.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/formats/metadata/SimpleSampleMetadata.java @@ -5,6 +5,8 @@ import org.broadinstitute.hellbender.utils.Utils; /** + * Metadata associated with a single sample. + * * @author Samuel Lee <slee@broadinstitute.org> */ public class SimpleSampleMetadata implements SampleMetadata { diff --git a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/models/AlleleFractionModeller.java b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/models/AlleleFractionModeller.java index 9187d13894b..560477979ee 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/models/AlleleFractionModeller.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/models/AlleleFractionModeller.java @@ -75,7 +75,7 @@ final class AlleleFractionModeller { Utils.nonNull(allelicCounts); Utils.nonNull(segments); Utils.validateArg(allelicCounts.getMetadata().getSequenceDictionary().equals(segments.getMetadata().getSequenceDictionary()), - "Metadata do not match."); + "Metadata of the allelic counts and the segments do not match."); Utils.nonNull(prior); metadata = allelicCounts.getMetadata(); diff --git a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/models/CopyRatioModeller.java b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/models/CopyRatioModeller.java index be837528ad0..4b19cf45162 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/models/CopyRatioModeller.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/models/CopyRatioModeller.java @@ -56,7 +56,7 @@ final class CopyRatioModeller { Utils.nonNull(copyRatios); Utils.nonNull(segments); Utils.validateArg(copyRatios.getMetadata().getSequenceDictionary().equals(segments.getMetadata().getSequenceDictionary()), - "Metadata do not match."); + "Metadata of the copy ratios and the segments do not match."); Utils.nonEmpty(segments.getRecords()); metadata = copyRatios.getMetadata(); diff --git a/src/main/java/org/broadinstitute/hellbender/utils/IntervalUtils.java b/src/main/java/org/broadinstitute/hellbender/utils/IntervalUtils.java index 07c725c9bfc..330ff4830e5 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/IntervalUtils.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/IntervalUtils.java @@ -1351,7 +1351,7 @@ public static Map> createO * The order of contigs/sequences in the dictionary is the order of the sorting here. * * @param dictionary dictionary to use for the sorting. Intervals with sequences not in this dictionary will cause - * exceptions to be thrown. Never {@ode null}. + * exceptions to be thrown. Never {@code null}. * @return an instance of {@code Comapator} for use in sorting of Locatables. */ public static Comparator getDictionaryOrderComparator(final SAMSequenceDictionary dictionary) { diff --git a/src/test/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleLocatableCollectionUnitTest.java b/src/test/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleLocatableCollectionUnitTest.java index f4ddf1d27ef..91fc81d6298 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleLocatableCollectionUnitTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/copynumber/formats/collections/AbstractSampleLocatableCollectionUnitTest.java @@ -18,6 +18,7 @@ import java.io.File; import java.io.IOException; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.function.BiConsumer; import java.util.function.Function; @@ -169,6 +170,7 @@ public void testReadMissingColumn() { public void testWrite() throws IOException { final File tempFile = createTempFile("test", ".tsv"); SIMPLE_LOCATABLE_COLLECTION_EXPECTED.write(tempFile); + SIMPLE_LOCATABLE_COLLECTION_EXPECTED.write(tempFile); //test that file is overwritten Assert.assertTrue(FileUtils.contentEquals(tempFile, SIMPLE_LOCATABLE_COLLECTION_FILE)); } @@ -202,6 +204,13 @@ public void testIntervalsWithOverlaps() { new SimpleSampleLocatableCollection(METADATA_EXPECTED, intervalsWithOverlaps); } + @Test(expectedExceptions = IllegalArgumentException.class) + public void testIntervalOutsideSequenceDictionary() { + final List intervalOutsideSequenceDictionary = Collections.singletonList( + new SimpleLocatable(new SimpleInterval("X", 1, 100), 1)); + new SimpleSampleLocatableCollection(METADATA_EXPECTED, intervalOutsideSequenceDictionary); + } + private static void assertSimpleLocatableCollectionEqualsExpected(final SimpleSampleLocatableCollection simpleLocatableCollection) { Assert.assertEquals(simpleLocatableCollection, SIMPLE_LOCATABLE_COLLECTION_EXPECTED); Assert.assertEquals(simpleLocatableCollection.getMetadata(), SIMPLE_LOCATABLE_COLLECTION_EXPECTED.getMetadata()); diff --git a/src/test/java/org/broadinstitute/hellbender/tools/copynumber/segmentation/AlleleFractionKernelSegmenterUnitTest.java b/src/test/java/org/broadinstitute/hellbender/tools/copynumber/segmentation/AlleleFractionKernelSegmenterUnitTest.java index c8168a30aa9..6022499612e 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/copynumber/segmentation/AlleleFractionKernelSegmenterUnitTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/copynumber/segmentation/AlleleFractionKernelSegmenterUnitTest.java @@ -63,7 +63,7 @@ public Object[][] dataAlleleFractionKernelSegmenter() { new SAMSequenceDictionary(intervals.stream() .map(SimpleInterval::getContig) .distinct() - .map(c -> new SAMSequenceRecord(c, 1000)) + .map(c -> new SAMSequenceRecord(c, 10000)) .collect(Collectors.toList()))); final int globalDepth = 100; diff --git a/src/test/java/org/broadinstitute/hellbender/tools/copynumber/segmentation/CopyRatioKernelSegmenterUnitTest.java b/src/test/java/org/broadinstitute/hellbender/tools/copynumber/segmentation/CopyRatioKernelSegmenterUnitTest.java index 33162030eb1..600cc64f6bd 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/copynumber/segmentation/CopyRatioKernelSegmenterUnitTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/copynumber/segmentation/CopyRatioKernelSegmenterUnitTest.java @@ -52,7 +52,7 @@ public Object[][] dataCopyRatioKernelSegmenter() { new SAMSequenceDictionary(intervals.stream() .map(SimpleInterval::getContig) .distinct() - .map(c -> new SAMSequenceRecord(c, 1000)) + .map(c -> new SAMSequenceRecord(c, 10000)) .collect(Collectors.toList()))); final CopyRatioCollection denoisedCopyRatios = new CopyRatioCollection(