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

[Breaking] for *_plot() and ecg_fixpeaks(method="neurokit") #889

Merged
merged 22 commits into from
Aug 23, 2023

Conversation

DominiqueMakowski
Copy link
Member

@DominiqueMakowski DominiqueMakowski commented Aug 22, 2023

  • Breaking: Now ecg_fixpeaks(method="neurokit") returns also a tuple with an info dict for consistency with kubios method
  • Breaking: ecg_plot() loses the show_type="artifacts" option
  • Breaking: *_plot() now take an "info" dict as input instead of the sampling_rate argument
  • ecg_plot improvements
    • Move the ax0 plot code of ecg_plot() into a separate internal function in ecg_peaks() + potential improvement of showing the "fixed" peaks (would also help removing the weird show_type option that we have in ecg_plot())
    • Add delineation plot in ecg_plot, as part of ecg_segment_plot code?
  • Add show methods (with ax arg) to ecg_rate, ecg_delineate, ecg_peaks that can then get called from ecg_plot() that would just to an assembling of existing plots, to remove code complexity and duplication
  • ecg_clean : method based on convolution of the simulated wavelet over the signal?

@DominiqueMakowski DominiqueMakowski changed the title [Feature] Add peak correction to PPG processing [Breaking] Now ecg_fixpeaks(method="neurokit") returns also a tuple with an info dict for consistency with kubios method Aug 23, 2023
@DominiqueMakowski DominiqueMakowski changed the title [Breaking] Now ecg_fixpeaks(method="neurokit") returns also a tuple with an info dict for consistency with kubios method [Breaking] ecg_fixpeaks(method="neurokit") also returns a tuple Aug 23, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2023

Codecov Report

Patch coverage: 77.21% and no project coverage change.

Comparison is base (6493c6e) 55.17% compared to head (b47335f) 55.17%.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #889   +/-   ##
=======================================
  Coverage   55.17%   55.17%           
=======================================
  Files         302      302           
  Lines       14049    14103   +54     
=======================================
+ Hits         7751     7781   +30     
- Misses       6298     6322   +24     
Files Changed Coverage Δ
neurokit2/ppg/ppg_segment.py 83.33% <ø> (ø)
neurokit2/signal/signal_fixpeaks.py 72.32% <39.62%> (-1.44%) ⬇️
neurokit2/ppg/ppg_plot.py 47.82% <58.33%> (-8.43%) ⬇️
neurokit2/eda/eda_plot.py 93.18% <75.00%> (-2.28%) ⬇️
neurokit2/emg/emg_plot.py 88.15% <77.77%> (-5.18%) ⬇️
neurokit2/rsp/rsp_plot.py 65.34% <77.77%> (-1.66%) ⬇️
neurokit2/ppg/ppg_peaks.py 88.88% <84.21%> (-11.12%) ⬇️
neurokit2/ecg/ecg_plot.py 87.50% <87.50%> (+2.50%) ⬆️
neurokit2/ecg/ecg_peaks.py 89.79% <88.37%> (-10.21%) ⬇️
neurokit2/ecg/ecg_segment.py 92.00% <93.75%> (+1.30%) ⬆️
... 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.

@DominiqueMakowski
Copy link
Member Author

New ecg_plot() with additional info

image

@DominiqueMakowski DominiqueMakowski changed the title [Breaking] ecg_fixpeaks(method="neurokit") also returns a tuple [Breaking] for *_plot() and ecg_fixpeaks(method="neurokit") Aug 23, 2023
@danibene
Copy link
Collaborator

  • Dynamic plots: @danibene instead of re-plotting with plotly, can't we just turn the existing matplotlib figs as interactive in htmls?

I did look into this at some point and I remember there was a library for this, I don't have time to find my old notebook right now, but I think it was https://mpld3.github.io/modules/API.html#mpld3.fig_to_html

@DominiqueMakowski DominiqueMakowski merged commit 4bf8071 into dev Aug 23, 2023
9 checks passed
@DominiqueMakowski DominiqueMakowski deleted the ppg_fixpeaks branch August 23, 2023 16:23
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