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

Fix unmapped reads in sequence dictionary #26

Conversation

carlyeks
Copy link
Member

This should fix the issue that @fnothaft described in an email; the test in e53ac85 fails in the same way (a NoSuchElementException); the changes in f47aa0c makes this test pass.

- Unmapped reads should be properly handled when loading from sequence dictionary
- Unmapped reads should not try to go into the sequence dictionary to
  get their information, nor even set their reference id/name
@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM/99/

@tdanford
Copy link
Contributor

These look good to me, Carl.

import org.apache.spark.rdd.RDD

class Bam2AdamSuite extends SparkFunSuite {
sparkTest("Bam2Adam should not fail on unmapped reads") {
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the test case to catch this in the future.

massie added a commit that referenced this pull request Dec 12, 2013
…equenceDictionary

Fix unmapped reads in sequence dictionary
@massie massie merged commit 9b1dd16 into bigdatagenomics:master Dec 12, 2013
@massie
Copy link
Member

massie commented Dec 12, 2013

Thanks, Carl!

@tdanford tdanford deleted the carlyeks/fixUnmappedReadsInSequenceDictionary branch December 13, 2013 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants