MIT-LCP / wfdb-python

Native Python WFDB package
MIT License
738 stars 300 forks source link

Use int64 instead of int for ann2rr #454

Closed bemoody closed 1 year ago

bemoody commented 1 year ago

This fixes issue #453 by using 64-bit integers on all platforms. (64-bit integers are way overkill for storing RR intervals, but I think it's wise to keep it simple and use 64-bit integers for all types of sample intervals.)

This is a pretty simple fix, but looking at the code I also notice there are a couple of significant issues with this function:

As a result, when you look at Lucas's example in the docstring:

    >>> wfdb.ann2rr('sample-data/100', 'atr', as_array=False)
    >>> 18
    >>> 59
    >>> ...
    >>> 250
    >>> 257

the first two "RR intervals" listed here are bogus and misleading: the first is the interval between the start of the record and the first RHYTHM annotation, and the second is the interval between the RHYTHM annotation and the first NORMAL annotation.

All that is by way of saying, I'd like to add a test case for this function but I think it's kind of broken to begin with and probably its behavior should be better nailed down first.

tompollard commented 1 year ago

Looks good to me, thanks Benjamin.