feat: honor original algorithm, increase test coverage

.github/workflows/unit-tests.yml

@@ -33,5 +33,5 @@ jobs:
       - name: Create Code Coverage Report
         if: matrix.java-version == '11'
         run: |
-          ./mill --no-server --disable-ticker ${{ matrix.suite }}.scoverage.xmlReport
+          ./mill --no-server --disable-ticker ${{ matrix.suite }}.scoverage.htmllsReport
       # TODO: build an HTML report and upload as an artifact

neodisambiguate/src/io/cvbio/neodisambiguate/MathUtil.scala

@@ -13,16 +13,6 @@ object MathUtil {
     def countMinBy[B](fn: A => B)(implicit cmp: Ordering[B]): Int = MathUtil.countMin(self.map(fn))
   }
 
-  /** Implicits for optionally finding the maximum and minimum items in a collection. */
-  implicit class WithMaxMinOption[A](self: IterableOnce[A]) {
-
-    /** Optionally return the maximum item from a container. */
-    def maxOption(implicit cmp: Ordering[A]): Option[A] = if (self.iterator.isEmpty) None else Some(self.iterator.max)
-
-    /** Optionally return the minimum item from a container. */
-    def minOption(implicit cmp: Ordering[A]): Option[A] = if (self.iterator.isEmpty) None else Some(self.iterator.min)
-  }
-
   /** Implicits picking the single maximum or single minimum item in a collection. */
   implicit class WithPickMaxMinBy[A](self: Seq[A]) {
 

neodisambiguate/src/io/cvbio/neodisambiguate/TemplateOrdering.scala

@@ -30,11 +30,15 @@ object TemplateOrdering extends FgBioEnum[TemplateOrdering] {
 
     /** Compare two templates using the original published algorithm. */
     override def compare(x: Template, y: Template): Int = {
-      val alignmentScore = (template: Template) => MetricPair[Int](template, AS.toString)(_ max _)
-      val numMismatches  = (template: Template) => MetricPair[Int](template, NM.toString)(_ min _)
+      val bestAlignmentScore  = (template: Template) => MetricPair[Int](template, AS)(_ max _)
+      val worstAlignmentScore = (template: Template) => MetricPair[Int](template, AS)(_ min _)
+      val bestNumMismatches   = (template: Template) => MetricPair[Int](template, NM)(_ min _)
+      val worstNumMismatches  = (template: Template) => MetricPair[Int](template, NM)(_ max _)
 
-      var compare = alignmentScore(x).compare(alignmentScore(y))
-      if (compare == 0) compare = -numMismatches(x).compare(numMismatches(y)) // Negate because less is better.
+      var compare = bestAlignmentScore(x).compare(bestAlignmentScore(y))
+      if (compare == 0) worstAlignmentScore(x).compare(worstAlignmentScore(y))
+      if (compare == 0) compare = -bestNumMismatches(x).compare(bestNumMismatches(y))   // Negate because less is better.
+      if (compare == 0) compare = -worstNumMismatches(x).compare(worstNumMismatches(y)) // Negate because less is better.
       compare
     }
   }

neodisambiguate/src/io/cvbio/neodisambiguate/metric/MetricPair.scala

@@ -1,6 +1,7 @@
 package io.cvbio.neodisambiguate.metric
 
 import com.fulcrumgenomics.bam.Template
+import htsjdk.samtools.SAMTag
 import io.cvbio.neodisambiguate.CommonsDef.SamTag
 import io.cvbio.neodisambiguate.bam.Bams.ReadOrdinal.{Read1, Read2}
 import io.cvbio.neodisambiguate.bam.Bams.TemplateUtil
@@ -33,6 +34,11 @@ object MetricPair {
     )
   }
 
+  /** Build a [[MetricPair]] from a [[Template]]. A function is required to reduce the tag values to one canonical value. */
+  def apply[T](template: Template, tag: SAMTag)(fn: (T, T) => T)(implicit cmp: Ordering[T]): MetricPair[T] = {
+    apply(template = template, tag = tag.toString)(fn = fn)
+  }
+
   /** Build an empty [[MetricPair]]. */
   def empty[T](implicit cmp: Ordering[T]): MetricPair[T] = new MetricPair[T](read1 = None, read2 = None)
 }

neodisambiguate/test/src/io/cvbio/neodisambiguate/bam/BamsTest.scala

@@ -33,6 +33,12 @@ class BamsTest extends UnitSpec {
     iterator.hasNext shouldBe false
   }
 
+  it should "raise an exception when empty and next() is called" in {
+    val builder  = new SamBuilder(sort = Some(SamOrder.Queryname))
+    val iterator = Bams.templatesIterator(builder.toSource)
+    an[NoSuchElementException] shouldBe thrownBy { iterator.next() }
+  }
+
   it should "accept a single valid SamSource with more than zero alignments" in {
     val builder  = new SamBuilder(sort = Some(SamOrder.Queryname))
     builder.addPair(name = "pair1", start1 = 1, start2 = 2)