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

More options with NK #8

Open
DominiqueMakowski opened this issue Mar 21, 2021 · 2 comments
Open

More options with NK #8

DominiqueMakowski opened this issue Mar 21, 2021 · 2 comments
Assignees
Labels
Discussion Discussion of a concept or implementation. Need to stay always open.

Comments

@DominiqueMakowski
Copy link

Hey folks, I took a quick look at the code here, and f I'm not wrong the 3 main functions that could benefit from NK's methods would be filter_physio(), interpolate_physio() and peakfind_physio().

What you could do if you don't want to have NK as a dependency is to have it as a optional dependency, i.e., add an argument like method = "peakdet" (in which case it would do what it currently does), and if method is something else then it tries to load NK (see a usage example here).

For the peak detection method, we could outsource it (or duplicate it if you go for optional dependency) in NK either as a new ecg peak detection method or as a more general agnostic peak detection method.

@DominiqueMakowski DominiqueMakowski added the Discussion Discussion of a concept or implementation. Need to stay always open. label Mar 21, 2021
@62442katieb
Copy link

Hey @DominiqueMakowski,

Thanks for taking a look! I think you're right, that seems like the best way to integrate our peak-finding methods. I think an optional NK dependency in peakdet sounds great.

Unfortunately I'm swamped finishing up some last-minute analyses for my dissertation 😅 so I won't be able to get to these changes for a couple weeks.

@me-pic
Copy link
Collaborator

me-pic commented Apr 14, 2023

Hello @62442katieb ! Trying to revive this issue. I'm going to also take a look at that for the rest of the codesprint. Let me know if you want to coordinate 😄

@m-miedema m-miedema transferred this issue from physiopy/peakdet Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Discussion of a concept or implementation. Need to stay always open.
Projects
Status: Todo
Development

No branches or pull requests

3 participants