-
Notifications
You must be signed in to change notification settings - Fork 12
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 error that occurs when encoding a value greater than 32767 with ‘B, S’ data type in encoding alignment in encoding alignment. #264
Fix error that occurs when encoding a value greater than 32767 with ‘B, S’ data type in encoding alignment in encoding alignment. #264
Conversation
Codecov Report
@@ Coverage Diff @@
## master #264 +/- ##
==========================================
+ Coverage 88.71% 88.87% +0.15%
==========================================
Files 78 78
Lines 6389 6389
Branches 442 442
==========================================
+ Hits 5668 5678 +10
+ Misses 279 269 -10
Partials 442 442
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for fixing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing this!
It seems that the BAM file you committed (test-resources/bam/include-BS-type-option.bam
) doesn't contain B:S
options. Would you replace it with a one generated by samtools view -bh test-resources/sam/include-BS-type-option.sam
?
95fdd56
to
fa167d3
Compare
@alumi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Thanks!
In encoding alignment options, encoding values greater than 32767 with the 'B, S' data type causes an error.
For example, encoding the following alignment would result in the following error message. Note that the alignment option has the value of '65535' for 'B, S' type.
q006 0 ref 10 60 16M * 0 0 AAAAGGGGTTTTCCCC * XS:B:S,0,32767,65535
The reason for error is as follows.
According to BAM format specification, the value of 'B, S' type are encoded with
uint16_t
, but the current code was processing withShort/parseShort
. Therefore, when encoding values greater than 32767,Short/parseShort
returns an error.This PR solves this problem by removing
Short/parseShort
and usinglsb/write-ushort
. This PR also adds tests to convert SAM/BAM file that contains the alignments with 'B, S' type data.