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

ReferenceRegion.apply(AlignmentRecord) should not NPE on unmapped reads #1099

Closed
fnothaft opened this Issue Aug 1, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@fnothaft
Member

fnothaft commented Aug 1, 2016

This is a little one that's bugged me for a long time. We should have a require that checks whether the read is mapped and throws a more informative error message.

@fnothaft fnothaft added the bug label Aug 1, 2016

@fnothaft fnothaft added this to the 0.20.0 milestone Aug 1, 2016

@fnothaft fnothaft self-assigned this Aug 1, 2016

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Aug 1, 2016

Member

Dupe of #821?

Member

heuermh commented Aug 1, 2016

Dupe of #821?

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Aug 1, 2016

Member

Nah, I would categorize #821 differently, since #821 is the join operation.

Member

fnothaft commented Aug 1, 2016

Nah, I would categorize #821 differently, since #821 is the join operation.

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Aug 1, 2016

Member

Also, #821 won't be resolved by the resolution to this. You'll still get an exception from calling ReferenceRegion.apply on an unmapped read, it'll just be a clearer exception.

Member

fnothaft commented Aug 1, 2016

Also, #821 won't be resolved by the resolution to this. You'll still get an exception from calling ReferenceRegion.apply on an unmapped read, it'll just be a clearer exception.

fnothaft added a commit to fnothaft/adam that referenced this issue Aug 13, 2016

fnothaft added a commit to fnothaft/adam that referenced this issue Aug 14, 2016

@heuermh heuermh closed this in ca05a86 Aug 25, 2016

heuermh added a commit to heuermh/adam that referenced this issue Sep 27, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment