Skip to content

Commit

Permalink
Fix AlignedSegment's pack_tags()'s exceptions
Browse files Browse the repository at this point in the history
There is no `array.typecode` class attribute, so the `array.array`
arm produced an AttributeError instead of the intended ValueError.
Perhaps `value.typecode` was intended; but this error only arises
when a `value_type` has been specified, so the array's typecode
has no influence on `typecode` anyway. So we simplify the message.

Also add a similar check in the scalar arm of this if-else to avoid
an invalid `typecode` here being reported as a raw KeyError.

Fixes #1233. Closes #1235 -- hat tip @marcus1487 for issue analysis.

[TODO] All this code is confusing and should be rewritten using
HTSlib's more recent aux field manipulation APIs.
  • Loading branch information
jmarshall committed Oct 11, 2024
1 parent 9a694d0 commit 7f83184
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
6 changes: 4 additions & 2 deletions pysam/libcalignedsegment.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ cdef inline pack_tags(tags):
raise ValueError("unsupported type code '{}'".format(value.typecode))

if typecode not in DATATYPE2FORMAT:
raise ValueError("invalid value type '{}' ({})".format(chr(typecode), array.typecode))
raise ValueError("invalid value type '{}'".format(chr(typecode)))

# use array.tostring() to retrieve byte representation and
# save as bytes
Expand All @@ -407,8 +407,10 @@ cdef inline pack_tags(tags):

if typecode == b'Z' or typecode == b'H':
datafmt = "2sB%is" % (len(value)+1)
else:
elif typecode in DATATYPE2FORMAT:
datafmt = "2sB%s" % DATATYPE2FORMAT[typecode][0]
else:
raise ValueError("invalid value type '{}'".format(chr(typecode)))

args.extend([pytag[:2],
typecode,
Expand Down
12 changes: 12 additions & 0 deletions tests/AlignedSegment_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1624,6 +1624,18 @@ def test_set_tag_with_automated_type_detection(self):
alt_value_type="I",
)

def test_set_tag_invalid_value_type(self):
with self.assertRaises(ValueError):
self.check_tag("TT", "abc", value_type="#")

def test_set_array_tag_invalid_value_type(self):
with self.assertRaises(ValueError):
self.check_tag("TT", array.array('I', range(4)), value_type='#')

def test_set_array_tag_invalid_typecode(self):
with self.assertRaises(ValueError):
self.check_tag("TT", array.array('L', range(4)), value_type=None)


class TestSetTagsGetTag(TestSetTagGetTag):
def check_tag(self, tag, value, value_type, alt_value_type=None):
Expand Down

0 comments on commit 7f83184

Please sign in to comment.