Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

finalizeRegion modifies read base qualities in place #6882

Closed
2 tasks done
DonFreed opened this issue Oct 11, 2020 · 1 comment · Fixed by #6943
Closed
2 tasks done

finalizeRegion modifies read base qualities in place #6882

DonFreed opened this issue Oct 11, 2020 · 1 comment · Fixed by #6943

Comments

@DonFreed
Copy link
Contributor

Bug Report

Affected class

AssemblyBasedCallerUtils

Affected version(s)

  • Latest public release version 4.1.9.0
  • Latest master branch as of 10/10/2020

Description

When adjusting the base quality of overlapping read pairs, the modifications are made in place. If the modified reads are later used in another active region, the results from the later active region will be changed by the earlier modification.

We had previously fixed this issue in #4926. But it looks like the refactoring in 1353e32 reverted to the earlier behavior.

AssemblyBasedCallerUtilsUnitTest.testfinalizeRegion() will fail due to this behavior if line 67 is changed from:

        AssemblyBasedCallerUtils.finalizeRegion(activeRegion, false, false, minbq, header, sampleList, false);

to:

        AssemblyBasedCallerUtils.finalizeRegion(activeRegion, false, false, minbq, header, sampleList, true);
@droazen
Copy link
Contributor

droazen commented Oct 20, 2020

@davidbenjamin It seems like your PR #6358 may have had the side effect of reverting the fix in #4926 -- mind having a look when you get a chance?

@droazen droazen assigned jamesemery and unassigned davidbenjamin Nov 6, 2020
droazen pushed a commit that referenced this issue Nov 24, 2020
…valid base qualities (#6943)

This fixes the test that was accidentally disabled (when we added a flag for overlapping read adjustment we forgot to set it correctly for the test) and reinstated something like the code from #4926. It should now necessarily be the case that the finalize() method calls read.copy() at least once for every read (though many multiples of once are still quite possible).

Fixes #6882
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants