FIX: Read Nihon Kohden annotation file accurately#13251
FIX: Read Nihon Kohden annotation file accurately#13251larsoner merged 9 commits intomne-tools:mainfrom
Conversation
|
The tests failed because: I saw a note here: There are only two events in the .LOG file, but four annotations in the .EDF file. So I wrote a test script: # encoding: utf-8
import os.path
import urllib.request
import edfio
from mne.io.nihon import read_raw_nihon
def download_file(url: str, output_path: str):
try:
urllib.request.urlretrieve(url, output_path)
return True
except Exception as e:
print(e)
return False
def test_edf():
file = r"MB0400FU.EDF"
if not os.path.exists(file):
if not download_file("https://raw.githubusercontent.com/mne-tools/mne-testing-data/refs/heads/master/NihonKohden/MB0400FU.EDF",
file):
return
edf = edfio.read_edf(file)
for annotation in edf.get_annotations():
print(annotation)
def test_eeg():
eeg_file = "MB0400FU.EEG"
elec_file = "MB0400FU.21E"
pnt_file = "MB0400FU.PNT"
log_file = "MB0400FU.LOG"
if not os.path.exists(eeg_file):
if not download_file("https://raw.githubusercontent.com/mne-tools/mne-testing-data/refs/heads/master/NihonKohden/MB0400FU.EEG",
eeg_file):
return
if not os.path.exists(elec_file):
if not download_file("https://raw.githubusercontent.com/mne-tools/mne-testing-data/refs/heads/master/NihonKohden/MB0400FU.21E",
elec_file):
return
if not os.path.exists(pnt_file):
if not download_file("https://raw.githubusercontent.com/mne-tools/mne-testing-data/refs/heads/master/NihonKohden/MB0400FU.PNT",
pnt_file):
return
if not os.path.exists(log_file):
if not download_file("https://raw.githubusercontent.com/mne-tools/mne-testing-data/refs/heads/master/NihonKohden/MB0400FU.LOG",
log_file):
return
raw = read_raw_nihon(eeg_file)
for onset, duration, description in zip(
raw.annotations.onset,
raw.annotations.duration,
raw.annotations.description,
):
print(onset, description)
if __name__ == "__main__":
test_edf()
test_eeg()Output: |
|
It appears that MB0400FU.EDF is suspicious. Therefore, I used nk2edf to convert By the way, the https://gitlab.com/Teuniz/EDFbrowser/-/blob/master/edf_annotations.cpp?ref_type=heads#L157 When reading MB0400FU_1-1+.zip using |
|
I also noticed that Nihon Kohden's software supports a type of annotation called |
|
Therefore, I believe this test failure should be addressed by updating MB0400FU.EDF in the following way:
|
fe60298 to
2265c3d
Compare
df1d550 to
9805b69
Compare
* upstream/main: (46 commits) MAINT: Restore edfio git install (mne-tools#13421) Support preload=False for the new EEGLAB single .set format (mne-tools#13096) [pre-commit.ci] pre-commit autoupdate (mne-tools#13453) MAINT: Restore PySide6 6.10.0 testing (mne-tools#13446) MAINT: Auth [skip azp] [skip actions] MAINT: Deploy [circle deploy] [skip azp] [skip actions] Bump github/codeql-action from 3 to 4 in the actions group (mne-tools#13442) ENH: Dont constrain fiducial clicks to mesh vertices (mne-tools#13445) Use timezone-aware ISO 8601 for website timestamp (mne-tools#13347) [pre-commit.ci] pre-commit autoupdate (mne-tools#13443) FIX: Update osf.io links to new format (mne-tools#13440) MAINT: Ensure full checkout is used (mne-tools#13439) Add BDF export (mne-tools#13435) [pre-commit.ci] pre-commit autoupdate (mne-tools#13434) [pre-commit.ci] pre-commit autoupdate (mne-tools#13431) MAINT: Update code credit (mne-tools#13432) FIX, TST: Try to get test_export_epochs_eeeglab passing again (mne-tools#13428) FIX: Add on_few_samples parameter to core rank estimation (mne-tools#13350) MAINT: Reenable mpl nightly (mne-tools#13426) [pre-commit.ci] pre-commit autoupdate (mne-tools#13427) ...
There was a problem hiding this comment.
Sorry we missed this one @myd7349 ! In the future if we don't respond in a week or so feel free to ping us again for review, this one just slipped through the cracks
I pushed a couple tiny commits to add your test file from option (1) above, bump testing version, adjust the test appropriately, and adjust a couple tiny style things. Marking for merge-when-green, thanks in advance @myd7349 !
* upstream/main: (23 commits) ENH: Add on_missing for combine_channels (mne-tools#13463) Bump the actions group with 2 updates (mne-tools#13464) Move development dependencies into a dependency group (no more extra) (mne-tools#13452) ENH: add on_missing for rename_channels (mne-tools#13456) add advisory board to website (mne-tools#13462) ENH: Support Nihon Kohden EEG-1200A V01.00 (mne-tools#13448) MAINT: Update dependency specifiers (mne-tools#13459) ENH: Add encoding parameter to Nihon Kohden reader (mne-tools#13458) [MAINT] Automatic SPEC0 dependency version management (mne-tools#13451) FIX: Read Nihon Kohden annotation file accurately (mne-tools#13251) MAINT: Restore edfio git install (mne-tools#13421) Support preload=False for the new EEGLAB single .set format (mne-tools#13096) [pre-commit.ci] pre-commit autoupdate (mne-tools#13453) MAINT: Restore PySide6 6.10.0 testing (mne-tools#13446) MAINT: Auth [skip azp] [skip actions] MAINT: Deploy [circle deploy] [skip azp] [skip actions] Bump github/codeql-action from 3 to 4 in the actions group (mne-tools#13442) ENH: Dont constrain fiducial clicks to mesh vertices (mne-tools#13445) Use timezone-aware ISO 8601 for website timestamp (mne-tools#13347) [pre-commit.ci] pre-commit autoupdate (mne-tools#13443) ...
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

Reference issue (if any)
Fix #11267.
What does this implement/fix?
This PR adds support for reading sub event log blocks in Nihon Kohden EEG annotation files (.LOG).
In certain versions of the .LOG files, in addition to the standard event log blocks, there are sub event log blocks.
HHMMSSformat.cccuuu.Additional information
I noticed that @jacobshaw42 also attempted something similar in #11431, but for some reason, the PR was closed.
This PR differs from #11431 in the following ways:
Sub event log blocks are not limited to 'EEG-1200A V01.00'
For example, in MB0400FU.EEG, the device type is
EEG-1100C V01.00, yet it does contain sub event log blocks.Nihon Kohden does not clearly specify which device types or software versions generate .LOG files that include sub event log blocks. I previously tried to implement a function to determine whether sub event log blocks are present, based on the device type:
However, I found this approach overly complicated, so I switched to a more general strategy.
In Nihon Kohden .LOG files, the control block can define up to 43 event log blocks. When sub event blocks are present:
Therefore, this PR assumes that sub event log blocks are present when the number of log blocks (n_logblocks) parsed from the control block does not exceed 21.
BTW, in nk2edf, the presence of sub event log blocks is assumed.
Since the logic for reading event blocks and sub event blocks is largely similar, I refactored the relevant code in
_read_nihon_annotationsinto a helper function_read_event_log_block. Two conditions are used to ensure a sub event block is valid:Decode event description at last
Because event text can be split across the event log block and the sub event log block, this PR concatenates the byte strings from both blocks before decoding. This affects the following logic:
strptimemethod can no longer be used to parse theHHMMSStime(which is a byte string, not astr). Instead, the time is parsed using int for each component.