Skip to content

Commit 8dc6371

Browse files
committed
fix: sort aggregated pileup IDs numerically instead of lexicographically
Compound breakpoint IDs were sorted as strings, causing non-intuitive ordering (e.g. "112_456_5_7_8_9" instead of "5_7_8_9_112_456"). Change to numeric sorting via sortBy(_.toInt) for consistency.
1 parent f1f39a4 commit 8dc6371

File tree

2 files changed

+7
-7
lines changed

2 files changed

+7
-7
lines changed

src/main/scala/com/fulcrumgenomics/sv/tools/AggregateSvPileup.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,8 @@ case class AggregatedBreakpointPileup(id: String,
294294
right_targets: Option[String] = None
295295
) extends Metric with LazyLogging {
296296

297-
/** Returns the IDs of constituent breakpoints */
298-
def pileupIds(): Seq[String] = id.split("_").toSeq.sorted
297+
/** Returns the IDs of constituent breakpoints, sorted numerically */
298+
def pileupIds(): Seq[String] = id.split("_").toSeq.sortBy(_.toInt)
299299

300300
/** Combines the comma-delimited list of target strings when aggregating the `left_targets` and `right_target` fields. */
301301
private def combineTargetStrings(first: Option[String], second: Option[String]): Option[String] = {
@@ -317,7 +317,7 @@ case class AggregatedBreakpointPileup(id: String,
317317
assert(pileup.right_strand == right_strand)
318318

319319
AggregatedBreakpointPileup(
320-
id = pileupIds().appended(pileup.id).sorted.mkString("_"),
320+
id = pileupIds().appended(pileup.id).sortBy(_.toInt).mkString("_"),
321321
category = category,
322322
left_contig = left_contig,
323323
left_min_pos = Math.min(left_min_pos, pileup.left_pos),

src/test/scala/com/fulcrumgenomics/sv/tools/AggregateSvPileupTest.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ class AggregateSvPileupTest extends UnitSpec {
274274
)
275275

276276
val expAgg1 = AggregatedBreakpointPileup(
277-
id = "112_456_5_7_8_9",
277+
id = "5_7_8_9_112_456",
278278
category = "Inter-contig rearrangement",
279279
left_contig = "chr1",
280280
left_min_pos = 100,
@@ -363,7 +363,7 @@ class AggregateSvPileupTest extends UnitSpec {
363363
)
364364

365365
val expAgg1 = AggregatedBreakpointPileup(
366-
id = "10_7_8_9",
366+
id = "7_8_9_10",
367367
category = "Inter-contig rearrangement",
368368
left_contig = "chr1",
369369
left_min_pos = 400,
@@ -426,7 +426,7 @@ class AggregateSvPileupTest extends UnitSpec {
426426
)
427427

428428
val expAgg = AggregatedBreakpointPileup(
429-
id = "10_7_8_9",
429+
id = "7_8_9_10",
430430
category = "Inter-contig rearrangement",
431431
left_contig = "chr1",
432432
left_min_pos = 400,
@@ -462,7 +462,7 @@ class AggregateSvPileupTest extends UnitSpec {
462462
)
463463

464464
val expAgg = AggregatedBreakpointPileup(
465-
id = "10_7_8_9",
465+
id = "7_8_9_10",
466466
category = "Inter-contig rearrangement",
467467
left_contig = "chr1",
468468
left_min_pos = 400,

0 commit comments

Comments
 (0)