neuropsychology / NeuroKit

NeuroKit2: The Python Toolbox for Neurophysiological Signal Processing
https://neuropsychology.github.io/NeuroKit
MIT License
1.58k stars 421 forks source link

[Fix] refactor ecg_findpeaks #973

Closed danibene closed 7 months ago

danibene commented 8 months ago

Description

This PR aims to refactor ecg_findpeaks() following a failing codeclimate check: https://codeclimate.com/github/neuropsychology/NeuroKit/pull/926

Avoid deeply nested control flow statements.

                    for missed_peak in missed_peaks:
                        if missed_peak - peaks[idx[-2]] > int(0.36 * sampling_rate) and ma[missed_peak] > 0.5 * th:
                            insort(QRS, missed_peak)
                            break

Severity: Major Found in neurokit2/ecg/ecg_findpeaks.py - About 45 mins to fix

Proposed Changes

I changed the ecg_findpeaks_hamilton() function so that it is broken up into smaller functions. I also added a regression test for it.

Checklist

Here are some things to check before creating the PR. If you encounter any issues, do let us know :)

DominiqueMakowski commented 8 months ago

Nice, though if need be we can also just discard that check on codeclimate - I wonder whether you have access to codeclimate's NK checks if you log in with ur GH account

danibene commented 8 months ago

Couldn't find that option - would it make sense to update the config file in the repository?

DominiqueMakowski commented 7 months ago

Couldn't find that option - would it make sense to update the config file in the repository?

Yeah to be fair I think we should drop codeclimate entirely... it was kinda useful years ago but now with all the GH actions not so much

DominiqueMakowski commented 7 months ago

can I merge this?