Neurokit methods documentation and contributing questions

[raimonpv]

Question and context I'm working on a project in which I need to analyze long ECG data from inpatients in cardiac floors. I've been using your package for the last few weeks and I found that in my database, the best algorithm for R peak detection is the neurokit one, do you have any publication where it is explained the methodology behind?

About contributing, I have few questions:

• If I found typos errors and outdated docstrings, should I create first and issue and then open a PR or I can open directly a PR?
• Have you considered using pre-commit to unify the checks and run them easier? I can implement that if you wish
• The line length defined in CONTRIBUTING section differs from the one defined in the checks.

It is possible that I have to implement some additional features so I'll let you know what I do to see if it is from your interest.

Thanks for your awesome work!

[DominiqueMakowski]

the best algorithm for R peak detection is the neurokit one

This is consistent with the preliminary results obtained in this benchmarking (currently in the dev version of the docs).

This algorithm was implemented by @JanCBrammer so he'll be the best able to give more info. In general, I do believe that this information (the algorithm(s) and the benchmarking) should be published so that there is a document with more details and data that can be referenced. Since Jan is the main person behind this aspect, it depends a lot on him, and I'm not sure what his plans are and how it fits into his timeline.

Regarding contributing:

If I found typos errors and outdated docstrings, should I create first and issue and then open a PR or I can open directly a PR?

Create directly a PR. The way I see it, issues would be more to open the discussion about something that one is not sure if it's a good idea or when one is not sure how to implement something. But if you have specific changes in mind and are confident that they are relevant, then no need to bother, just PR directly (also, no need to write a big and over-detailed PR description if it's like typos fix or something straightforward). In the end, the contributing guidelines should be there precisely to avoid burdening contributors (and maintainers/reviewers as well).

Have you considered using pre-commit to unify the checks and run them easier? I can implement that if you wish

For that for instance we could open an issue to discuss it specifically. I'm not fully aware of what pre-commits are but it sounds very interesting! please tell us more :)

The line length defined in CONTRIBUTING section differs from the one defined in the checks.

Yeah... the reason is that I'm personally still hesitating about what is the optimal line length. I find 80 to be too narrow and an overall outdated convention in 2020 where most people have wide format screens. 100 seems to be alright but sometimes it's not enough especially for URL links, so I don't know whether to go with 100 or 120 🤷‍♂️. We should organize a vote 😅

It seems like github's width is around 124 or something like that, but just saw this in which (on my screen) it seems to be more towards 100 for linked code. So maybe we should stick with 100.

It is possible that I have to implement some additional features so I'll let you know what I do to see if it is from your interest.

Looking forward to your ideas and features ☺️ 🎉

[JanCBrammer]

@raimonpv, great to hear that the algorithm does a good job on your data! As @DominiqueMakowski mentioned, there isn't a paper that describes the algorithm (yet). Basically it simply detects QRS complexes based on the slope and then searches the R-peaks within those QRS complexes. It's quite straightforward actually (about 30 lines of Python): https://github.com/neuropsychology/NeuroKit/blob/cb37d83ee20d6a13a91c4848aa435f41e979e203/neurokit2/ecg/ecg_findpeaks.py#L207 If you want, you could visualize the thresholds by running ecg_findpeaks() with show=True. This should give you a pretty good idea of what is going on.

@DominiqueMakowski, I think we should publish the benchmarking results at some point. We can then explain the NeuroKit detector in that publication. Unfortunatly, I cannot prioritize this atm. The whole open source / biosignal thing is a side project (I'm using it as part of my PhD, but it's definitly not (supposed to be) the main aspect :grin:).

Looking forward to your ideas and features ☺️ 🎉

@raimonpv, agreed! Would be nice to have you around.

[DominiqueMakowski]

@DominiqueMakowski, I think we should publish the benchmarking results at some point. We can then explain the NeuroKit detector in that publication

Yeah, I think that makes sense. It's not our priority either at the moment, hopefully sooner or later we will manage to find time - or someone with a bit more time to help out - to put it in shape.

Unfortunatly, I cannot prioritize this atm. The whole open source / biosignal thing is a side project (I'm using it as part of my PhD, but it's definitly not (supposed to be) the main aspect 😁).

been there, I feel u bro 😅

[stale[bot]]

This issue has been automatically marked as inactive because it has not had recent activity. It will eventually be closed if no further activity occurs.

[marcoistasy]

hello! i'm just following up on this - has there been any progress? How would I go about reading data in Neurokit? There is no mention of in the documentation.

[JanCBrammer]

@marcoistasy, could you open a separate issue about "reading data"? I suspect you need to read data from a specific file format / device?

[marcoistasy]

hi

@marcoistasy, could you open a separate issue about "reading data"? I suspect you need to read data from a specific file format / device?

hi @JanCBrammer! of course, I have opened the issue (#393).

[stale[bot]]

This issue has been automatically marked as inactive because it has not had recent activity. It will eventually be closed if no further activity occurs.

[stale[bot]]

This issue has been inactive for a long time. We're closing it (but feel free to reopen it if need be).