Skip to content

MAINT: Replace manual PNS binary block reader in _read_segment_file with mffpy#14030

Open
PragnyaKhandelwal wants to merge 4 commits into
mne-tools:mainfrom
PragnyaKhandelwal:egi-mff-pns-reader
Open

MAINT: Replace manual PNS binary block reader in _read_segment_file with mffpy#14030
PragnyaKhandelwal wants to merge 4 commits into
mne-tools:mainfrom
PragnyaKhandelwal:egi-mff-pns-reader

Conversation

@PragnyaKhandelwal

Copy link
Copy Markdown
Contributor

Reference issue (if any)

Part of #13926 (Phase 2 — PNS binary block reader migration).

What does this implement/fix?

Replaces the ~65-line manual binary PNS block reader in RawMff._read_segment_file
(built around _block_r / np.fromfile loops) with mffpy-backed reading via
get_physical_samples_from_epoch(channels=["PNSData"]), consistent with how EEG
data has already been read since #13932.

Key changes:

  • PNS data is now fetched through the same _disk_range_to_epochs + mffpy epoch
    window mechanism used for EEG, removing ~65 lines of manual seek/read logic.
  • _block_r is no longer imported into egimff.py (it remains in general.py
    where it is still used for PNS header validation in _read_mff_header).
  • The PSG off-by-one sample quirk (EGI hardware occasionally stores one fewer PNS
    sample than EEG in the last block) is handled with cols[:pns_chunk.shape[1]],
    matching the intent of the previous samples_to_read == 1 guard.

Additional information

I used Claude Sonnet 4.6 to assist with diagnosing two bugs caught during testing
(a missing _mult_cal_one calibration call and the PSG shape mismatch). I reviewed,
understood, and tested all changes myself.

… with `mffpy`-backed reading via `get_physical_samples_from_epoch`
@PragnyaKhandelwal PragnyaKhandelwal marked this pull request as ready for review July 7, 2026 10:10
@PragnyaKhandelwal

Copy link
Copy Markdown
Contributor Author
image

Test failure seems unrelated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant