add new feature AHP_depth_from_AP_begin_voltage (and slow version)

[arnaudon]

[arnaudon]

@AurelienJaquier , just give me general feedback on that, I'll add tests/docs as for the other one later

[arnaudon]

@AurelienJaquier , good to review!

[codecov-commenter]

Codecov Report

Merging #277 (c4f964e) into master (7024a95) will increase coverage by 0.14%. The diff coverage is 100.00%.

:exclamation: Current head c4f964e differs from pull request most recent head 7e86d09. Consider uploading reports for the commit 7e86d09 to get more accurate results

@@            Coverage Diff             @@
##           master     #277      +/-   ##
==========================================
+ Coverage   75.61%   75.76%   +0.14%     
==========================================
  Files          12       12              
  Lines        2990     3000      +10     
==========================================
+ Hits         2261     2273      +12     
+ Misses        729      727       -2     

Impacted Files	Coverage Δ
efel/tests/test_basic.py	99.56% <100.00%> (+<0.01%)	:arrow_up:

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[arnaudon]

@AurelienJaquier I found the issue here, it was that the implementation of AHP_depth_slow was ignoring the first and last APs, I'm not sure why at all. Just playing with indices now makes more sense, as it returns the values for the same AP as AHP_depth or AP_begin_voltage.

[AurelienJaquier]

I don't think you should change AHP_depth_slow behavior. Ignoring the first peak is not a bug, it is a feature. I think the reason is because we can encounter traces with an early peak before even the stimulus start.

If you want AHP_depth_slow to be able to also catch the 1st ISI, please make it dependent on the ignore_first_ISI setting.

[arnaudon]

mmh I'm not sure this is the reason, as this feature start detecting APs after stim_start, no? it should catch the same AP as AHP_depth IMO, if we set the delay to 0ms, then AHP_depth should be == to AHP_depth_slow, no?

[AurelienJaquier]

It isn't bound by stim start. But yes, it would be more coherent if AHP_depth_slow and AHP_depth followed the same logic

[arnaudon]

Ah the peak_indices is not between stim_start and stim_end? it should no? what is the reason for that? do you know?

[AurelienJaquier]

I would guess it's because for some cases we want to know about peaks outside stimulus, like rebound, etc.

[arnaudon]

I tried to do AHP_depth_abs_slow more like AHP_depth_abs, to be continued