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

0.2.7 #901

Merged
merged 37 commits into from
Nov 6, 2023
Merged

0.2.7 #901

merged 37 commits into from
Nov 6, 2023

Conversation

DominiqueMakowski
Copy link
Member

@DominiqueMakowski DominiqueMakowski commented Sep 12, 2023

Roadmap

RSP

  • Add rsp_segment() (similar to ppg_segment())
  • Add the cycle overlay plots to RSP
  • Streamline rsp_plot() (same as ppg_plot())

PPG

  • Detect PPG diastolic peaks? (between each peak and the next trough)
  • Add PPG amplitude calculation (reduction can be indicative of vasoconstriction in stress, cold)
  • Document peak-extraction methods (PPG)
  • add more PPG detectors (@peterhcharlton)
  • hrv_rsa(): Add method argument to compute only one RSA signal. Do something with warning that will be thrown when using bio_process() on signals < 32 sec where it will complain that the signal is too short to compute RSA with one of these methods.

Misc

  • we should also do a global search of " ``...()`` " and replace by " :func:`...` " to have hyperlinks in the docs across functions
  • Dynamic plots: @danibene instead of re-plotting with plotly, can't we just turn the existing matplotlib figs as interactive in htmls?
  • Fix ECG Analysis bug for sample rate of 500 #593

Complexity

DominiqueMakowski and others added 7 commits September 22, 2023 10:10
Fix a bug where you had an exception if there was not the same number of peaks and onsets.
This can happen on signal that has been cut (ex: depending on difference conditions).
The fix just ignores the first peak (only for display) when there is no onset for the first peak.
Compute the mean peaks amplitude only on the peaks (and not on the whole signal)
[Fix] eda_plot throws ValueError when the number of onsets is not the same as the number of peaks
[Feature] complexity_lyapunov(): more efficient method to compute LLE
@pull-request-size pull-request-size bot added size/XL and removed size/L labels Oct 2, 2023
@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2023

Codecov Report

Attention: 142 lines in your changes are missing coverage. Please review.

Comparison is base (ce33299) 55.19% compared to head (c1e38e1) 54.87%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #901      +/-   ##
==========================================
- Coverage   55.19%   54.87%   -0.33%     
==========================================
  Files         302      303       +1     
  Lines       14143    14255     +112     
==========================================
+ Hits         7806     7822      +16     
- Misses       6337     6433      +96     
Files Coverage Δ
neurokit2/__init__.py 85.36% <100.00%> (ø)
neurokit2/complexity/__init__.py 100.00% <100.00%> (ø)
neurokit2/complexity/entropy_approximate.py 47.82% <100.00%> (ø)
neurokit2/complexity/entropy_sample.py 90.00% <100.00%> (ø)
neurokit2/complexity/utils_entropy.py 81.03% <100.00%> (ø)
neurokit2/ecg/ecg_clean.py 75.60% <100.00%> (ø)
neurokit2/ecg/ecg_peaks.py 80.30% <ø> (ø)
neurokit2/ecg/ecg_plot.py 89.65% <ø> (ø)
neurokit2/hrv/intervals_process.py 90.00% <100.00%> (ø)
neurokit2/complexity/entropy_multiscale.py 55.10% <83.33%> (+0.84%) ⬆️
... and 10 more

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mperreir and others added 5 commits October 3, 2023 09:41
Added a test to avoid a "RuntimeWarning: Mean of empty slice" for intervals where no peak is detected
[Fix] `SCR_Peaks_Amplitude_Mean` should be only computed on peaks
@DominiqueMakowski
Copy link
Member Author

@danibene the docs CI flagged the intervals_process() function in hrv/intervals_process, but looking at it 1) I'm super suprised it hasn't been detected before because the example likely never ran; 2) the example seems weird; 3) it makes me wonder whether this should even be an exported function, or whether we should turn it into an internal _preprocess_rpeaks_intervals()

any thoughts?

@danibene
Copy link
Collaborator

danibene commented Oct 9, 2023

@danibene the docs CI flagged the intervals_process() function in hrv/intervals_process, but looking at it 1) I'm super suprised it hasn't been detected before because the example likely never ran; 2) the example seems weird; 3) it makes me wonder whether this should even be an exported function, or whether we should turn it into an internal _preprocess_rpeaks_intervals()

any thoughts?

  1. yeah I'm not sure either why it's being flagged now
  2. what about the example seems weird? Maybe we could show how it can be used prior to the hrv functions? (So that the R-R intervals are given as an input?)
  3. I don't think so, this would then prevent processing the R-R intervals prior to time-domain or nonlinear HRV feature computation. Though admittedly, this is probably not well known/documented enough for users to be using it this way

To provide more context, in kubios the R-R intervals are detrended https://www.kubios.com/hrv-preprocessing/

Copy link

codeclimate bot commented Nov 2, 2023

Code Climate has analyzed commit c1e38e1 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@DominiqueMakowski DominiqueMakowski merged commit 3d004b4 into master Nov 6, 2023
10 checks passed
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.

5 participants