Thanks; this looks good to me.

It looks to me like the existing code in XQRS._is_twave was buggy (wfdb/processing/qrs.py, lines 531-545). If I understand correctly, scipy.preprocessing.normalize would return an Nx1 array, np.diff would return an Nx0 array, the < comparison would return an empty one-dimensional array, so the function would always return false.

I would assume that the new version is in fact what @cx1111 originally intended, and hopefully should improve the overall specificity of the algorithm. But, Chen, do you think this is okay or should be tested more?

For the old version of XQRS, over mitdb:

• Gross QRS sensitivity: 99.35
• Gross QRS positive predictivity: 99.74
• Average QRS sensitivity: 99.34
• Average QRS positive predictivity: 99.71

For the new version:

• Gross QRS sensitivity: 99.03
• Gross QRS positive predictivity: 99.76
• Average QRS sensitivity: 99.08
• Average QRS positive predictivity: 99.73

(Total false positives decreased from 237 to 216, false negatives increased from 592 to 888.)

Not great but not terrible, and might be useful sometimes.

The old behavior is equivalent to setting the config parameter t_inspect_period to zero:

qrs_inds = xqrs_detect(sig, fs, conf=XQRS.Conf(t_inspect_period=0))

So if we wanted to keep the old behavior by default we could just set the default t_inspect_period to 0.

Thanks @hofaflo. Nice checks @bemoody and I think it would make sense to set the default as you suggest. Please could someone take a look at doing this? Once the default is set, it sounds like we should be good to merge.

Thanks for the benchmark @bemoody!
Done @tompollard.

thanks @hofaflo, looks good to me! okay to merge @bemoody?

Thanks!