Skip to content

Commit

Permalink
chore: fixup the verbs in the API a bit
Browse files Browse the repository at this point in the history
  • Loading branch information
@clintval
clintval committed Jan 17, 2022
1 parent 6d2a5b2 commit 55e7238
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 44 deletions.
16 changes: 8 additions & 8 deletions src/main/scala/com/fulcrumgenomics/bam/pileup/ByCoordinatePileupBuilder.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class ByCoordinatePileupBuilder private(
includeSecondaryAlignments = includeSecondaryAlignments,
includeSupplementalAlignments = includeSupplementalAlignments,
excludeMapPositionOutsideFrInsert = excludeMapPositionOutsideFrInsert,
) with Closeable { // TODO: One could extend AbstractIterator[Pileup[PileupEntry]] and scan the loci one-by-one as well!
) with Closeable {
import com.fulcrumgenomics.bam.pileup.ByCoordinatePileupBuilder.LocatablePileup

/** Whether this builder is able to pileup more records from the input iterator of SAM records or not. */
Expand All @@ -78,7 +78,7 @@ class ByCoordinatePileupBuilder private(
private var lastPileup: Option[Pileup[PileupEntry]] = None

/** Advance this builder to the next requested locus and add all possibly overlapping records to the cache. */
@inline private def advanceTo(refIndex: Int, pos: Int): Unit = {
@inline private def seek(refIndex: Int, pos: Int): Unit = {
// Drop records up until the next record stands a chance of overlapping the requested locus. Then, take records up
// until the next record stands a chance of having a start greater than the requested locus plus one. All records in
// this query have a chance of overlapping the locus so we must then filter down to only those that have an end
Expand All @@ -95,15 +95,15 @@ class ByCoordinatePileupBuilder private(
/** A genomic ordering for any locatable that utilizes the sequence dictionary corresponding to the input records. */
private lazy val byCoordinate: Ordering[Locatable] = GenomicOrdering(dict)

/** Records that we've accumulated that could overlap another coordinate-advancing call to <skipTo>. */
/** Records that we've accumulated that could overlap another coordinate-advancing call to <advanceTo>. */
private lazy val cache: mutable.ArrayBuffer[SamRecord] = new mutable.ArrayBuffer[SamRecord](initialCacheSize)

/** The underlying buffered stream of input SAM records which is lazily summoned. */
private lazy val underlying: Iterator[SamRecord] = records.filter(_.mapped).bufferBetter

/** Efficiently skip to the next coordinate-maintaining or coordinate-advancing locus and build a pileup there. */
def skipTo(refName: String, pos: Int): Pileup[PileupEntry] = {
require(!closed, "Cannot skip to a new locus if the pileup builder was closed!")
/** Efficiently advance to the next coordinate-maintaining or coordinate-advancing locus and build a pileup there. */
def advanceTo(refName: String, pos: Int): Pileup[PileupEntry] = {
require(!closed, "Cannot advance to a new locus if the pileup builder was closed!")
val currentLocus = new Interval(refName, pos, pos)
val refIndex = dict(refName).index.ensuring(_ >= 0, s"Reference name not in sequence dictionary: $refName")

Expand All @@ -119,7 +119,7 @@ class ByCoordinatePileupBuilder private(
case Some(last) if byCoordinate.lt(last, currentLocus) =>
lastPileup = None // Set to `None` now so we can drop the object reference ASAP for garbage collection.
cache.filterInPlace(rec => (rec.refIndex == refIndex && rec.end >= pos) || rec.refIndex > refIndex)
this.advanceTo(refIndex = refIndex, pos = pos)
this.seek(refIndex = refIndex, pos = pos)
this.build(cache, refName = refName, pos = pos)
case Some(last) if byCoordinate.equiv(last, currentLocus) => last
case Some(last) =>
Expand All @@ -128,7 +128,7 @@ class ByCoordinatePileupBuilder private(
s"greater than or equal to: ${last.refName}:${last.pos}"
)
case None =>
this.advanceTo(refIndex = refIndex, pos = pos)
this.seek(refIndex = refIndex, pos = pos)
this.build(cache, refName = refName, pos = pos)
}

Expand Down
13 changes: 8 additions & 5 deletions src/main/scala/com/fulcrumgenomics/bam/pileup/PileupBuilder.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class PileupBuilder(
if (compare) compare = rec.end >= pos
if (compare) compare = rec.start <= pos || startsWithInsertion
if (compare) compare = if (excludeMapPositionOutsideFrInsert && rec.isFrPair) {
PileupBuilder.positionInsideFrInsert(rec, refName = refName, pos = pos).contains(true)
PileupBuilder.positionInsideFrInsert(rec, refIndex = refIndex, pos = pos).contains(true)
} else { compare }
compare
}
Expand All @@ -140,11 +140,14 @@ class PileupBuilder(
/** Companion object for [[PileupBuilder]]. */
object PileupBuilder {

/** Returns true if the position <refName>:<pos> is inside the insert of <rec>, if <rec> is in a mapped FR pair. */
private def positionInsideFrInsert(rec: SamRecord, refName: String, pos: Int): Option[Boolean] = {
Option.when(rec.refName == refName && rec.isFrPair) {
/** Returns true if <rec> is in a mapped FR pair and the position <pos> is inside the insert coordinates of <rec>.
* Returns false if <rec> is in a mapped FR pair but the position <pos> is outside the insert coordinates of <rec>.
* Returns None if <rec> is not in a mapped FR pair regardless of where <pos> is located.
*/
private def positionInsideFrInsert(rec: SamRecord, refIndex: Int, pos: Int): Option[Boolean] = {
Option.when(rec.isFrPair) {
val (start, end) = Bams.insertCoordinates(rec)
pos >= start && pos <= end
rec.refIndex == refIndex && pos >= start && pos <= end
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/com/fulcrumgenomics/vcf/filtration/FilterSomaticVcf.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ import scala.collection.immutable.ListMap
val updated = SomaticFilters.filter(_.appliesTo(variant.genotypes(name))) match {
case Nil => variant
case filters =>
val pileup = if (streamBam) { piler.skipTo(refName = variant.chrom, pos = variant.pos) } else {
val pileup = if (streamBam) { piler.advanceTo(refName = variant.chrom, pos = variant.pos) } else {
val overlapping = records.query(variant.chrom, start = variant.pos, end = variant.pos, Overlapping)
piler.build(overlapping, refName = variant.chrom, pos = variant.pos)
}
Expand Down
60 changes: 30 additions & 30 deletions src/test/scala/com/fulcrumgenomics/bam/pileup/ByCoordinatePileupBuilderTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {
source.safelyClose()
}

"ByCoordinatePileupBuilder.skipTo" should "build a simple pileup of only matched/mismatched fragment reads" in {
"ByCoordinatePileupBuilder.advanceTo" should "build a simple pileup of only matched/mismatched fragment reads" in {
val builder = new SamBuilder(readLength = ReadLength, sd = Some(TestSequenceDictionary), sort = Some(Coordinate))

1 to 5 foreach { _ => builder.addFrag(start = 5).foreach(_.bases = "A" * ReadLength) }
Expand All @@ -77,7 +77,7 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {

val source = builder.toSource
val piler = ByCoordinatePileupBuilder(source, mappedPairsOnly = false)
val pile = piler.skipTo(Chr1, 5).withoutIndels
val pile = piler.advanceTo(Chr1, 5).withoutIndels

pile.depth shouldBe 5 + 4 + 3 + 2 + 1
pile.count(_.base == 'A') shouldBe 5
Expand Down Expand Up @@ -106,7 +106,7 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {

val source = builder.toSource
val piler = ByCoordinatePileupBuilder(source, mappedPairsOnly = false)
val pile = piler.skipTo(Chr2, 55).withoutIndels
val pile = piler.advanceTo(Chr2, 55).withoutIndels

pile.depth shouldBe 5 + 4 + 3 + 2 + 1
pile.count(_.base == 'A') shouldBe 5
Expand All @@ -131,18 +131,18 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {
val piler = ByCoordinatePileupBuilder(source, mappedPairsOnly = false)

// Before any of the indels, we should have 4 _bases_
piler.skipTo(Chr1, 105).baseIterator.size shouldBe 4
piler.advanceTo(Chr1, 105).baseIterator.size shouldBe 4

// The locus before the first insertion and deletion (should include the insertion)
val p1 = piler.skipTo(Chr1, 110)
val p1 = piler.advanceTo(Chr1, 110)
p1.depth shouldBe 4
p1.iterator.size shouldBe 5
p1.baseIterator.map(_.base.toChar).toSeq.sorted.mkString shouldBe "ACGT"
p1.iterator.collect{ case x: InsertionEntry => x }.map(_.rec.name).next() shouldBe "q2"
p1.baseIterator.toSeq should contain theSameElementsAs p1.withoutIndels.iterator.toSeq

// The locus of the first deleted base
val p2 = piler.skipTo(Chr1, 111)
val p2 = piler.advanceTo(Chr1, 111)
p2.depth shouldBe 4
p2.iterator.size shouldBe 4
p2.baseIterator.size shouldBe 3
Expand All @@ -151,7 +151,7 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {
p2.baseIterator.toSeq should contain theSameElementsAs p2.withoutIndels.iterator.toSeq

// The locus of the bigger indels
val p3 = piler.skipTo(Chr1, 131)
val p3 = piler.advanceTo(Chr1, 131)
p3.depth shouldBe 4
p3.iterator.size shouldBe 5
p3.baseIterator.size shouldBe 3
Expand All @@ -161,7 +161,7 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {
p3.baseIterator.toSeq should contain theSameElementsAs p3.withoutIndels.iterator.toSeq

// Locus where there is a read with a leading indel
val p4 = piler.skipTo(Chr1, 140)
val p4 = piler.advanceTo(Chr1, 140)
p4.depth shouldBe 4 // shouldn't count the leading indel
p4.iterator.size shouldBe 5
p4.baseIterator.size shouldBe 4
Expand All @@ -181,11 +181,11 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {
val source = builder.toSource
val piler = ByCoordinatePileupBuilder(source, mappedPairsOnly = false)

val pile1 = piler.skipTo(Chr1, 5).withoutIndels
val pile1 = piler.advanceTo(Chr1, 5).withoutIndels
pile1.depth shouldBe 1
pile1.count(_.base == 'N') shouldBe 1

val pile2 = piler.skipTo(Chr1, 5).withoutIndels
val pile2 = piler.advanceTo(Chr1, 5).withoutIndels
pile2.depth shouldBe 1
pile2.count(_.base == 'N') shouldBe 1

Expand All @@ -201,11 +201,11 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {
val source = builder.toSource
val piler = ByCoordinatePileupBuilder(source, mappedPairsOnly = false)

val pile1 = piler.skipTo(Chr1, 6).withoutIndels
val pile1 = piler.advanceTo(Chr1, 6).withoutIndels
pile1.depth shouldBe 1
pile1.count(_.base == 'N') shouldBe 1

an[IllegalArgumentException] shouldBe thrownBy { piler.skipTo(Chr1, 5) }
an[IllegalArgumentException] shouldBe thrownBy { piler.advanceTo(Chr1, 5) }

source.safelyClose()
piler.safelyClose()
Expand All @@ -220,7 +220,7 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {

val source = builder.toSource
val piler = ByCoordinatePileupBuilder(source, minMapQ = 10, mappedPairsOnly = false)
val pile = piler.skipTo(Chr1, 105)
val pile = piler.advanceTo(Chr1, 105)

pile.depth shouldBe 2
pile.exists(_.rec.name == "q1") shouldBe false
Expand All @@ -246,7 +246,7 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {

val source = builder.toSource
val piler = ByCoordinatePileupBuilder(source, mappedPairsOnly = false)
val pile = piler.skipTo(Chr1, 105)
val pile = piler.advanceTo(Chr1, 105)

pile.depth shouldBe 2
pile.exists(_.rec.name == "q1") shouldBe false
Expand All @@ -264,7 +264,7 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {

val source = builder.toSource
val piler = ByCoordinatePileupBuilder(source)
val pile = piler.skipTo(Chr1, 105)
val pile = piler.advanceTo(Chr1, 105)

pile.depth shouldBe 1
pile.exists(r => r.rec.name == "q3" && r.rec.firstOfPair) shouldBe true
Expand All @@ -282,7 +282,7 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {

val source = builder.toSource
val piler = ByCoordinatePileupBuilder(source)
val pile = piler.skipTo(Chr1, 125)
val pile = piler.advanceTo(Chr1, 125)

pile.depth shouldBe 5
pile.withoutOverlaps.depth shouldBe 3
Expand All @@ -300,10 +300,10 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {
val source = builder.toSource
val piler = ByCoordinatePileupBuilder(source)

piler.skipTo(Chr1, 100).depth shouldBe 0
piler.skipTo(Chr1, 101).depth shouldBe 2
piler.skipTo(Chr1, 100 + ReadLength - 1).depth shouldBe 2
piler.skipTo(Chr1, 101 + ReadLength - 1).depth shouldBe 0
piler.advanceTo(Chr1, 100).depth shouldBe 0
piler.advanceTo(Chr1, 101).depth shouldBe 2
piler.advanceTo(Chr1, 100 + ReadLength - 1).depth shouldBe 2
piler.advanceTo(Chr1, 101 + ReadLength - 1).depth shouldBe 0

source.safelyClose()
piler.safelyClose()
Expand All @@ -317,10 +317,10 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {
val source = builder.toSource
val piler = ByCoordinatePileupBuilder(source, excludeMapPositionOutsideFrInsert = false)

piler.skipTo(Chr1, 100).depth shouldBe 1
piler.skipTo(Chr1, 101).depth shouldBe 2
piler.skipTo(Chr1, 100 + ReadLength - 1).depth shouldBe 2
piler.skipTo(Chr1, 101 + ReadLength - 1).depth shouldBe 1
piler.advanceTo(Chr1, 100).depth shouldBe 1
piler.advanceTo(Chr1, 101).depth shouldBe 2
piler.advanceTo(Chr1, 100 + ReadLength - 1).depth shouldBe 2
piler.advanceTo(Chr1, 101 + ReadLength - 1).depth shouldBe 1

source.safelyClose()
piler.safelyClose()
Expand All @@ -334,10 +334,10 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {
val source = builder.toSource
val piler = ByCoordinatePileupBuilder(source)

piler.skipTo(Chr1, 100).depth shouldBe 1
piler.skipTo(Chr1, 101).depth shouldBe 2
piler.skipTo(Chr1, 100 + ReadLength - 1).depth shouldBe 2
piler.skipTo(Chr1, 101 + ReadLength - 1).depth shouldBe 1
piler.advanceTo(Chr1, 100).depth shouldBe 1
piler.advanceTo(Chr1, 101).depth shouldBe 2
piler.advanceTo(Chr1, 100 + ReadLength - 1).depth shouldBe 2
piler.advanceTo(Chr1, 101 + ReadLength - 1).depth shouldBe 1

source.safelyClose()
piler.safelyClose()
Expand All @@ -351,7 +351,7 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {
val source = builder.toSource

val piler1 = ByCoordinatePileupBuilder(source)
val pile1 = piler1.skipTo(Chr1, 105)
val pile1 = piler1.advanceTo(Chr1, 105)
val p1 = pile1.pile.head.asInstanceOf[BaseEntry]

pile1.depth shouldBe 1
Expand All @@ -363,7 +363,7 @@ class ByCoordinatePileupBuilderTest extends UnitSpec {
p1.positionInReadInReadOrder shouldBe 5

val piler2 = ByCoordinatePileupBuilder(source)
val pile2 = piler2.skipTo(Chr1, 205)
val pile2 = piler2.advanceTo(Chr1, 205)
val p2 = pile2.pile.head.asInstanceOf[BaseEntry]

pile2.depth shouldBe 1
Expand Down

0 comments on commit 55e7238

Please sign in to comment.