Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] detrend R-R intervals #711

Merged
merged 42 commits into from
Sep 28, 2022

Conversation

danibene
Copy link
Collaborator

Description

This PR aims to add the ability to detrend R-R intervals as a preprocessing step. See also #671 (comment)

Proposed Changes

I changed the intervals_preprocess() function so that it optionally calls signal_detrend() by passing detrend_method.

Checklist

  • 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)

danibene and others added 30 commits September 21, 2022 08:08
also added some comments
since intervals are meant to be more general (can be breath-to-breath)
Preprocessed intervals, in milliseconds.
intervals_time : array
Preprocessed timestamps corresponding to intervals, in seconds.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add some examples here?

@DominiqueMakowski
Copy link
Member

Awesome, last two little things; should we rename intervals_preprocess() to intervals_process() to maintain consistency with the other *_process() functions

Also, we can add th new exported function to the documentation

@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2022

Codecov Report

Base: 52.65% // Head: 52.74% // Increases project coverage by +0.09% 🎉

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

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #711      +/-   ##
==========================================
+ Coverage   52.65%   52.74%   +0.09%     
==========================================
  Files         277      279       +2     
  Lines       12588    12624      +36     
==========================================
+ Hits         6628     6659      +31     
- Misses       5960     5965       +5     
Impacted Files Coverage Δ
neurokit2/misc/__init__.py 100.00% <ø> (ø)
neurokit2/hrv/hrv.py 42.55% <50.00%> (+2.12%) ⬆️
neurokit2/hrv/hrv_rqa.py 46.15% <50.00%> (+8.65%) ⬆️
neurokit2/hrv/hrv_utils.py 61.84% <75.00%> (-0.48%) ⬇️
neurokit2/hrv/hrv_nonlinear.py 64.59% <78.94%> (-0.51%) ⬇️
neurokit2/hrv/intervals_process.py 81.25% <81.25%> (ø)
neurokit2/hrv/intervals_utils.py 81.48% <81.48%> (ø)
neurokit2/hrv/__init__.py 100.00% <100.00%> (ø)
neurokit2/hrv/hrv_frequency.py 60.86% <100.00%> (-0.84%) ⬇️
neurokit2/hrv/hrv_rsa.py 94.76% <100.00%> (+2.54%) ⬆️
... and 7 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
Copy link
Member

All good for me, feel free to merge!

@danibene
Copy link
Collaborator Author

All good for me, feel free to merge!

Awesome, is it fine for me to merge #710 first? Since this is based off of that one

@DominiqueMakowski
Copy link
Member

Yup!

@danibene danibene merged commit 87877c3 into neuropsychology:dev Sep 28, 2022
@danibene danibene deleted the feature/detrend_rri branch September 28, 2022 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants