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

improved checking for upstream deletions in GenotypingEngine #6429

Merged
merged 3 commits into from
Feb 25, 2020

Conversation

davidbenjamin
Copy link
Contributor

@ldgauthier Here's a GenotypeGVCFs bug fix for you.

@tfenne Your suspicion was correct.

Copy link
Contributor

@ldgauthier ldgauthier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor suggestion. As always, thanks for the elegant fix.

* Add deletions to a list.
* Record emitted deletion in order to remove downstream spanning deletion alleles that are not covered by any emitted deletion.
* In addition to recording a new deletion, this method culls previously-recorded deletions that end before the current variant
* context. This assumes that variants are traversed in order.
*
* @param deletionSize size of deletion in bases
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you note that this is the size for the normalized representation? Otherwise it's not obvious why we need this information in addition to the VC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really didn't need it. I rewrote the method to take in the VC and the emitted alleles.

@davidbenjamin davidbenjamin merged commit ca19a40 into master Feb 25, 2020
@davidbenjamin davidbenjamin deleted the db_stateful_deletion branch February 25, 2020 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants