Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Fix for https://github.com/broadinstitute/foghorn/issues/149 #385

Merged
merged 1 commit into from
Nov 21, 2014

Conversation

kgururaj
Copy link
Collaborator

Fix for https://github.com/broadinstitute/foghorn/issues/149
Check for missing variant in MultipleVariantIterator - important if one of the inputs is a blank VCF
TODO: Are there other places this check needs to be done?

@droazen
Copy link
Contributor

droazen commented Nov 18, 2014

Let's hold off on merging this fix until #384 is merged, as it conflicts with the changes in that branch.

@coveralls
Copy link

Coverage Status

Coverage increased (+4.47%) when pulling 92e28af on intel_multi_iterator_fix into a25b637 on master.

@droazen
Copy link
Contributor

droazen commented Nov 19, 2014

Now that #384 has been merged, this can be rebased on top of those changes (will probably get merge conflicts).

@jmthibault79
Copy link
Contributor

I would like to see a test which demonstrates the fix as well.

Thanks for looking into this btw!

@droazen
Copy link
Contributor

droazen commented Nov 21, 2014

What is the status of this?

@kgururaj
Copy link
Collaborator Author

Added a test case for empty VCF traversal using MVI. The test crashes without the fix.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 6ce9443 on intel_multi_iterator_fix into cc0a9d2 on master.

@droazen
Copy link
Contributor

droazen commented Nov 21, 2014

👍 looks good to me -- but @jmthibault79 should have a look too

@jmthibault79
Copy link
Contributor

Looks good but I would also like a copy of the test for MultipleVariantIterator (current one tests ReferenceBlockSplittingVariantIterator). OK to rebase and merge yourself when that is added.

of the inputs is a blank VCF

Added a simple test case for empty VCF traversal using MultipleVariantIterator
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 5b0e0f9 on intel_multi_iterator_fix into a2a227b on master.

@kgururaj kgururaj merged commit 857044d into master Nov 21, 2014
@kgururaj kgururaj deleted the intel_multi_iterator_fix branch November 21, 2014 22:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants