[Feature] R-R intervals and associated timestamps as input to HRV functions

[danibene]

Description

This PR aims at adding the ability to directly pass R-R intervals to the HRV functions. See also #684

Proposed Changes

Instead of separately sanitizing the input and converting to R-R intervals, I added a function _hrv_format_input() that can accept a dictionary with the keys RRI and RRI_Time and always returns the same format (either peaks or intervals depending on the HRV function) regardless of whether peaks or intervals are passed.

Checklist

Here are some things to check before creating the PR. If you encounter any issues, do let us know :)

• I have read the CONTRIBUTING file.
• My PR is targetted at the dev branch (and not towards the master branch).
• I ran the CODE CHECKS on the files I added or modified and fixed the errors.
• I have added the newly added features to News.rst (if applicable)

[codecov-commenter]

Codecov Report

Base: 52.65% // Head: 52.77% // Increases project coverage by +0.12% 🎉

Coverage data is based on head (67a5aee) compared to base (262cc00).
Patch coverage: 80.50% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #710      +/-   ##
==========================================
+ Coverage   52.65%   52.77%   +0.12%     
==========================================
  Files         277      278       +1     
  Lines       12588    12619      +31     
==========================================
+ Hits         6628     6660      +32     
+ Misses       5960     5959       -1     

Impacted Files	Coverage Δ
neurokit2/hrv/hrv.py	41.30% <40.00%> (+0.87%)	⬆️
neurokit2/hrv/hrv_rqa.py	46.15% <50.00%> (+8.65%)	⬆️
neurokit2/hrv/hrv_utils.py	65.34% <75.47%> (+3.02%)	⬆️
neurokit2/hrv/hrv_nonlinear.py	64.59% <78.94%> (-0.51%)	⬇️
neurokit2/misc/intervals_successive.py	90.90% <90.90%> (ø)
neurokit2/misc/intervals_to_peaks.py	94.11% <93.75%> (+44.11%)	⬆️
neurokit2/hrv/hrv_frequency.py	60.00% <100.00%> (-1.71%)	⬇️
neurokit2/hrv/hrv_rsa.py	94.73% <100.00%> (+2.51%)	⬆️
neurokit2/hrv/hrv_time.py	94.17% <100.00%> (-0.17%)	⬇️
neurokit2/misc/__init__.py	100.00% <100.00%> (ø)
... and 5 more

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[DominiqueMakowski]

I think the docstrings of intervals_to_peaks failed 🤔

[danibene]

I think the docstrings of intervals_to_peaks failed 🤔

I think I fixed it now!

Since there's a bit of repetition in the way the intervals are cleaned across functions (_hrv_sanitize_rri(), intervals_to_peaks()), thoughts on refactoring such that intervals is a separate subpackage containing e.g. intervals_to_peaks(), find_successive_intervals(), intervals_sanitize(), and intervals_preprocess()? Or should I just leave as is?

[DominiqueMakowski]

mmh a subpackage might be overkill, but we could put all these functions in separate files within the hrv module, since HRV is the one that predominantly deals with intervals?

• intervals_to_peaks()
• find_successive_intervals(): could be renamed intervals_successive() for consistency?
• intervals_sanitize()
• intervals_preprocess()

Or perhaps just putting these in separate files in the misc

[DominiqueMakowski]

What about merging intervals_successive with intervals_sanitize()? Or do you think there would be some usecase for intervals_successive() outside of intervals sanitization?

[danibene]

What about merging intervals_successive with intervals_sanitize()? Or do you think there would be some usecase for intervals_successive() outside of intervals sanitization?

I use it to adapt some hrv functions for missing data (e.g. only using truly successive differences in rmssd), so I would keep it as a separate function

[DominiqueMakowski]

If I understand you use it through _hrv_format_input which uses it via sanitize in _hrv_get_rri() anyway, so we could make it an internal function and put it in hrv_utils no?

(my thinking is that if a function is mostly used by us or rather than by users in their analysis, it can be good to make it an internal to prevent feature creep and avoid bloating the documentation - that's why I was asking for potential other use cases that users my have with this)

[danibene]

If I understand you use it through _hrv_format_input which uses it via sanitize in _hrv_get_rri() anyway, so we could make it an internal function and put it in hrv_utils no?

(my thinking is that if a function is mostly used by us or rather than by users in their analysis, it can be good to make it an internal to prevent feature creep and avoid bloating the documentation - that's why I was asking for potential other use cases that users my have with this)

Yes, but I also plan to use intervals_successive() within the hrv_time() and hrv_nonlinear() functions in the PR addressing #541 :

My current implementation is here: https://github.com/danibene/NeuroKit/blob/feature/adjust_for_missing_intervals/neurokit2/hrv/hrv_time.py#L168-L173

I can still make it an internal function though - should I move it to _intervals_successive() in hrv_utils?

[DominiqueMakowski]

okay cool

I can still make it an internal function though - should I move it to _intervals_successive() in hrv_utils?

let's do that for now (we can leave all the examples and descriptions), and down the road we can always make it user-facing if need be