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] ECG inversion #716

Merged
merged 15 commits into from
Oct 3, 2022
Merged

Conversation

danibene
Copy link
Collaborator

@danibene danibene commented Sep 29, 2022

Description

This PR aims at adding the ability to identify and correct ECG signal inversion. See also #558

Proposed Changes

I added the ecg_invert() function so that a user could optionally correct for signal inversion in the raw ECG data before using the other processing functions (cleaning, peak detection).

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
Copy link

codecov-commenter commented Oct 1, 2022

Codecov Report

Base: 52.72% // Head: 52.76% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (58fe039) compared to base (a811797).
Patch coverage: 72.41% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #716      +/-   ##
==========================================
+ Coverage   52.72%   52.76%   +0.03%     
==========================================
  Files         279      280       +1     
  Lines       12643    12672      +29     
==========================================
+ Hits         6666     6686      +20     
- Misses       5977     5986       +9     
Impacted Files Coverage Δ
neurokit2/ecg/ecg_peaks.py 100.00% <ø> (ø)
neurokit2/ecg/ecg_quality.py 26.31% <ø> (ø)
neurokit2/ecg/ecg_invert.py 71.42% <71.42%> (ø)
neurokit2/ecg/__init__.py 100.00% <100.00%> (ø)
neurokit2/eda/eda_eventrelated.py 81.63% <0.00%> (-2.05%) ⬇️

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.

danibene and others added 3 commits October 1, 2022 11:20
nevermind just realized that this would add an additional dependency, not worth the time saved I assume
@DominiqueMakowski
Copy link
Member

DominiqueMakowski commented Oct 2, 2022

I streamlined the example a bit and added a show option. Feel free to adjust/modify!

(Also, note that I added some

@suppress
plt.close()

After the plots: the reason for this is that basically, when sphinxruns the documentation and runs ipython on the example chuncks, it will create matplotlib figures. But it doesn't close them automatically, meaning that after 20 opened images it will error and say that there are too many images concurrently opened (and the documentation build will fail). So we need to close the plots that we create in the docstrings manually; and the @suppress tag is there to hide this line from the actual docs)

@danibene danibene linked an issue Oct 3, 2022 that may be closed by this pull request
@danibene
Copy link
Collaborator Author

danibene commented Oct 3, 2022

@DominiqueMakowski this all looks good to me, I'll go ahead and merge!

@danibene danibene closed this Oct 3, 2022
@danibene danibene reopened this Oct 3, 2022
@danibene danibene merged commit 026a4f0 into neuropsychology:dev Oct 3, 2022
@danibene danibene deleted the feature/ecg_invert branch October 3, 2022 14:27
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.

ECG peak detection robust to signal inversion
3 participants