Unexpected behaviour when array contains nans

[Andrea Mambrini]

Array without nans, peaks are correctly detected

In [30]: A = np.array([0,0,10, 0, 2, 0, 0])

In [31]: peakutils.indexes(A, thres=0, min_dist=1)
Out[31]: array([2, 4])

Same array, with nan at the end, no peaks are detected:

In [32]: A = np.array([0,0,10, 0, 2, 0, 0, np.nan])

In [33]: peakutils.indexes(A, thres=0, min_dist=1)
Out[33]: array([], dtype=int64)

Peakutils/numpy versions

In [35]: peakutils.__version__
Out[35]: '1.0.3'

In [36]: np.__version__
Out[36]: '1.11.0'

[Lucas Hermann Negri]

Hi,

I think that data with NaNs should be preprocessed according to each user intention before using peakutils. The fix I propose would be describing that pu doesn't handle NaNs and it is up to the user to prepare the data.

I'm compelled to do this as what would be expected in this case:

A = np.array([1,2,3,4,5,np.nan,3,2,1])
peakutils.indexes(A, thres=0, min_dist=1)

5 is not a peak since we don't know what value goes after it (it may keep increasing, just like when you have a maximum at the end of an array). 3 is also not a peak, for the same reason. One way would be filling the NaN with the average between 5 and 3, but the situation gets more complex when you have sequences of NaNs. There are some packages (scipy, sklearn,pandas, and even numpy) that have utilities related to filling NaNs.

What is your opinion?

[Lucas Hermann Negri]

Not a bug.

[Grégory Chatelier]

Not a bug but this could be handy directly in library since method returns indexes from given list we have to deal with NaNs, not just remove them.

Concerning numpy, NaNs can be converted to representation of -infinity : first we replace numpy.nan with -numpy.inf, then use numpy.nan_to_num() to get a large negative finite number

[Lucas Hermann Negri]

I agree. However, there isn't a standard way to handle NaNs: leaving this to the user removes magic from the method, and may prevent some hard to find bugs in user code.