bug: the logic for the PileupBuilder `includeMapPositionsOutsideFrInsert` filter is wrong

`positionIsOutsideFrInsert` is actually `positionIsInsideFrInsert`

https://github.com/fulcrumgenomics/fgbio/blob/3a74fd28ff630b417410953541ff107a1b7b0fb7/src/main/scala/com/fulcrumgenomics/bam/pileup/PileupBuilder.scala#L123-L128

The filter is used in a way that is reversed so that the current tests still work as designed.

https://github.com/fulcrumgenomics/fgbio/blob/3a74fd28ff630b417410953541ff107a1b7b0fb7/src/main/scala/com/fulcrumgenomics/bam/pileup/PileupBuilder.scala#L203-L205

Additionally there is a redundant `rec.isFrPair` call (this is done inside the method).

I came across this because I was tracking down some aberrant alleles in a pileup. Should reads mapped to separate chromosomes be rejected by this filter, or should that be a separate filter (likely in `quickAcceptRecord`)?  Currently these are accepted even if `includeMapPositionsOutsideFrInsert` is `false`, which i think may be misleading.

	private def positionIsOutsideFrInsert(rec: SamRecord, refIndex: Int, pos: Int): Boolean = {
	rec.isFrPair && {
	val (start, end) = Bams.insertCoordinates(rec)
	rec.refIndex == refIndex && pos >= start && pos <= end
	}
	}

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bug: the logic for the PileupBuilder `includeMapPositionsOutsideFrInsert` filter is wrong #980

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (compare) compare = if (!includeMapPositionsOutsideFrInsert && rec.isFrPair) {
	PileupBuilder.positionIsOutsideFrInsert(rec, refIndex = refIndex, pos = pos)
	} else { compare }

bug: the logic for the PileupBuilder includeMapPositionsOutsideFrInsert filter is wrong #980

Description