-
-
Notifications
You must be signed in to change notification settings - Fork 413
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] PPG plots improvements #883
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## dev #883 +/- ##
==========================================
+ Coverage 55.12% 55.17% +0.05%
==========================================
Files 298 301 +3
Lines 13956 14024 +68
==========================================
+ Hits 7693 7738 +45
- Misses 6263 6286 +23
☔ View full report in Codecov by Sentry. |
neurokit2/ecg/ecg_plot.py
Outdated
# fig_beats = ecg_segment( | ||
# ecg_signals["ECG_Clean"], peaks, sampling_rate, show="return" | ||
# ) | ||
# ax2 = fig_beats.axes[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now ecg_segment(..., show="return")
returns a Figure, but how can we insert that figure into the axis so that we don't have to re-plot it? 🤔 @danibene any ideas from the top of your head?
matplotlib is the worse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not off the top of my head, if I understood your question correctly it seems like it's not possible to insert axes from another figure according to the answer here: https://stackoverflow.com/questions/66369315/how-to-assign-axes-from-one-figure-to-axes-from-another-figure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok thanks, that was helpful, went with the ax
kwarg similar to other packages
@danibene I'm not sure how / if we can transfer that to the dynamic plots, but I'd say let's merge for now and then we'll see for the rest |
Breaking
ecg_plot()
andppg_plot()
now require the sampling_rate to be passed (for segmentation to happen) - also simplifies the code.Changes
ppg_peaks()
master function to bring it up-to-speed with ECGppg_segment()
to be able to segment individual peaks and nicely visualize themppg_plot()
by adding the individual peaks overlays to the plot (like ECG)