Strange filter order expected by ecg_preprocess about neurokit.py

[DominiqueMakowski]

I guess this part was based on biosppy, that multiples 0.3 by the sampling rate internally (hence the 0.3 as a default order).

However, this might be correct with only particular filters (guess I never thought about this as I never had the occasion of using different filters). Maybe we could allow the order parameter to be something like order="default", which would then be set appropriately (to some standard values depending on the filter type)?

What do you think?

[renatosc]

Sorry for taking too long to reply. I looked the biosppy code and it appears to me that they added that 0.3 multiplication line just as simplification to internally calculate a order based on the sampling rate provided since the order is not a parameter itself. In our case, the ecg_preprocess accepts order as a parameter (filter_order) and have it multiplied by the sampling rate does not appear correct to me. But I am no expert in DSP and also a change in that point could break code of users. What do you think? Leave that way?

[DominiqueMakowski]

Hum, the idea was just to expose the order parameter (further adjusted based on the sampling rate). Instead of being hard-coded to 0.3 as biosppy, it is set as default to 0.3 but can be changed if need be. To be honest, I never had to change this parameter, but I guess that for some other filter types it could make sense to change it.
Do you know how we could change it for the better?

[renatosc]

The problem is not the default value, but what you do with the value after:
order = int(filter_order * sampling_rate)

Because right now if I enter, let's say, filter_order = 4, the code will actually use 4 * sampling rate for the order, which is not intuitive from the outside.

[DominiqueMakowski]

Right, would you suggest then improving the documentation (specifying that the order value is further multiplied by the sampling rate), or changing the way it is handled internally?

[renatosc]

That is actually the question that I asked you...hehe. The old dilema of making a breaking change or not... I think for now the documentation note would be enough. That way we don't create a incompatible update without previous notification.