DOC: clarify average EEG reference behavior and update tutorial note#13664
DOC: clarify average EEG reference behavior and update tutorial note#13664Farzah11 wants to merge 4 commits intomne-tools:mainfrom
Conversation
mne/_fiff/reference.py
Outdated
| channel (or was removed during acquisition or preprocessing), it is not | ||
| implicitly accounted for in the average. | ||
| For sensor-space analyses where this distinction matters, consider adding | ||
| a zero-filled reference channel using :func:`add_reference_channel` before |
There was a problem hiding this comment.
this will need to be a fully qualified path or else the cross-reference will fail, e.g. :func:`~mne.add_reference_channels`
| # Note: | ||
| # When computing an average reference, if the dataset does not include | ||
| # the original reference electrode, the average will be biased because | ||
| # the missing channel is ignored. To avoid this, first add a | ||
| # zero-valued reference channel using | ||
| # mne.add_reference_channels(). This ensures that | ||
| # all intended channels, including the original reference, | ||
| # contribute correctly to the average. |
There was a problem hiding this comment.
as written, this will get treated like code comments. To format it like a normal paragraph, it needs # %% as the first line of the block. You could also consider making it a true "note" admonition, using .. note:: as the first line and indenting the rest.
|
Hi @drammock , I’ve pushed updates addressing the points you mentioned. I noticed that the build_docs and linkcheck checks are currently failing, Kindly review when convenient. Thank you:) |
|
for now I've manually triggered a re-run. |
|
lots of doc build failures: https://app.circleci.com/pipelines/gh/mne-tools/mne-python/30250/workflows/f842ecd6-5a62-44e1-b8be-f23114f9087a/jobs/79140?invite=true#step-143-0_106 @Farzah11 I recommend building docs locally to debug these quicker |
mne/_fiff/reference.py
Outdated
| in the data. If the original reference electrode was not recorded as a | ||
| channel (or was removed during acquisition or preprocessing), it is not | ||
| implicitly accounted for in the average. | ||
| For sensor-space analyses where this distinction matters, consider adding |
There was a problem hiding this comment.
I think this is not definite enough. Maybe:
This matters for sensor-space analyses: The resulting reference would not be a correct average reference, as the subtracted reference signal would be divided by n-channels and not n-channels + 1 reference channel. Further discussion can be found in Kim et. al 2013 https://doi.org/10.3389/frsip.2023.1064138. Consider adding a ...
|
Thanks for the clarification and for manually re-running the checks. |
- Move average reference note to docs.py (set_eeg_reference_see_also_notes) - Remove floating text from reference.py docstring - Add note to tutorial about using add_reference_channels for correct avg ref - Include Kim et al. (2023) paper reference - Fix line lengths to comply with PEP 8 (<= 79 chars)
Farzah11
force-pushed
the
fix-eeg-reference-doc
branch
from
dec3ef8 to
f44ea8f
Compare
|
Hi, I’ve been investigating the From the CircleCI page, it appears the jobs are currently blocked with Could a maintainer please approve or rerun the CircleCI workflow for this PR when convenient? Thank you. |
Hi Farzah, Could you explain what went wrong during your attempt at running the documentation locally? I had some issues on the weekend with building docs on a windows 11 server in python 3.11, so I might be able to help. Please also let us know if the documentation is unclear or missing something. Thank you for contributing, Carina |
|
Hi Carina, I built the docs locally on Windows 11 / Python 3.11 using make html and make linkcheck, and didn’t encounter a hard Sphinx error locally. At the moment I’m not seeing a specific CI failure yet, as the CircleCI jobs appear to be blocked with block-unregistered-user, so the docs build doesn’t actually run or produce logs. Once the checks are approved and run, I should be able to see any concrete errors and debug them properly. Thanks for the offer to help. |
|
@Farzah11 Please see this comment and make an account with CircleCI so that the jobs don't need to be manually triggered by us each time: #13664 (comment) But there is an issue with the changed docstring, as shown by this test failure: https://github.com/mne-tools/mne-python/actions/runs/22285364149/job/64470384061?pr=13664 |
|
Hi, I’ve created a CircleCI account as suggested. The CI jobs are still blocked with block-unregistered-user, so I believe my GitHub account needs to be approved for CircleCI on the MNE-Python side. @drammock Could you please approve my GitHub account for CI so the jobs can run automatically? In parallel, I’m fixing the docstring-related test failure shown in the GitHub Actions log and will push an update shortly. Thanks! |
@Farzah11 You need to push a commit (even an empty one) to trigger another CI run. |
5 participants
This PR clarifies the behavior of average EEG referencing when the original
reference electrode is not present in the data.
set_eeg_referencedocstring explaining thatthe average is computed only over existing EEG channels.
add_reference_channelwhen applying an average reference and the originalreference is missing.
Closes #13618