improvement ideas for Reports #13161

Activity

1. drammock

   added the

   Feature

   issue type on Mar 13, 2025
2. larsoner

   added

   EASY

   on Apr 2, 2026
3. Master-Zero1 commented on May 28, 2026

   

   Master-Zero1

   on May 28, 2026

   More actions

   Hi!!
   I would like to work on this

   My approach:

   1. Add a module-level _calculate_estimated_heartrate(events, sfreq) helper that computes BPM from R-peaks using median R-R interval, with physiological range filtering (0.3–2.0s) before computing
   2. In _add_projs(), capture the raw instance before it gets stripped to an Info object, detect ECG projectors by checking proj['desc'], call find_ecg_events() if ECG projectors are present, and pass the result as the figure caption
   3. Wrapping detection in try/except so it degrades gracefully if no ECG channel exists

   I have prior experience building an ECG denoising pipeline with R-peak detection and BPM estimation on raw noisy signals, so I'm comfortable with the edge cases here.

   Could I be assigned on this? Happy to open a draft PR.
4. larsoner commented on Jun 1, 2026

   

   larsoner

   on Jun 1, 2026

   Member

   More actions

   Hmmm that sounds a bit complex. Typically when someone adds ECG projectors to a report, they have already done the ECG event detection step. We don't want to re-run it.

   Instead, looking at

   https://mne.tools/stable/generated/mne.Report.html#mne.Report.add_projs

   We currently allow info : mne.Info | mne.Evoked. If we were to change this to also allow mne.Epochs -- like the kind you'd get from mne.create_ecg_epochs (or created manually after using something like mne.find_ecg_events, we could use the min and max times of the epochs instance, plus the len(epochs.drop_log) and epochs.info["sfreq"] to come up with a decent (not exact but presumably close enough) estimate of the detected heart rate, conditional on the projector name containing something about heart rate.

   Incidentally, I also use something like this for blinks, and the same process could apply. So we probably want something like both support for supplying mne.Epochs, but also a add_rate="auto" | bool where True means "estimate and add the event rate", False means don't add it, and "auto" means "True if the projector supplied are blink or ecg projectors, according to standard naming"
5. Master-Zero1 commented on Jun 4, 2026

   

   Master-Zero1

   on Jun 4, 2026

   Last edited by Master-Zero1

   More actions

   Sorry for the late reply .

   Thanks for the guidance, that makes much more sense! Re-running event detection is unnecessary if epochs are already available. I'll rework the approach accordingly.

   Updated plan:

   1. Accept mne.Epochs as inst in add_projs
   2. Use len(epochs.drop_log) and epochs.info["sfreq"] along with the min/max of epoch times to estimate the rate
   3. Add an add_rate="auto" | True | False parameter, where "auto" checks the projector name for ECG/EOG and only shows the rate if relevant
   4. Apply the same logic for blinks (EOG) as you mentioned

   I'll open a draft PR shortly — happy to get early feedback on the implementation before finalising.

   👍React with 👍1Reacted by Eric Larson
6. Master-Zero1 commented on Jun 5, 2026

   

   Master-Zero1

   on Jun 5, 2026

   More actions

   Hi @larsoner, I have a working implementation - Epochs support and add_rate parameter are in place and the report generates successfully.

   One thing I noticed while testing: when projectors are calculated via compute_proj_epochs the descriptions are named for the channel type and time window (e.g. planar-999--0.499-0.499-PCA-01) instead of having 'ecg' or 'eog' in them. So checking projector descriptions for those keywords doesn’t work reliably for the 'auto' mode.

   Are you looking for another field in the projector dict for the 'auto' detection, or did you have some other way you wanted to detect ECG/EOG projectors?
7. larsoner commented on Jun 5, 2026

   

   larsoner

   on Jun 5, 2026

   Member

   More actions

   One thing I noticed while testing: when projectors are calculated via compute_proj_epochs the descriptions are named for the channel type and time window (e.g. planar-999--0.499-0.499-PCA-01) instead of having 'ecg' or 'eog' in them.

   Yes when using compute_proj_epochs you have to add an appropriate desc_prefix. If it's ECG or EOG, the user should ideally put that in there. (If we don't do that in a tutorial, we should fix it!) The projectors computed by compute_proj_ecg / compute_proj_eog will have it in there. So I thnik it's okay for "auto" only to work if this has been set properly.