FIX, TST: Try to get test_export_epochs_eeeglab passing again by scott-huberty · Pull Request #13428 · mne-tools/mne-python

scott-huberty · 2025-09-23T00:09:18Z

Needs Handle special cases from MNE-Python jackz314/eeglabio#18

This test has been marked as failing for a while now:

mne-python/mne/export/tests/test_export.py

Lines 539 to 559 in 936bfae

    
           def test_export_epochs_eeglab(tmp_path, preload): 
        
               """Test saving an Epochs instance to EEGLAB's set format.""" 
        
               eeglabio = pytest.importorskip("eeglabio") 
        
               raw, events = _get_data()[:2] 
        
               raw.load_data() 
        
               epochs = Epochs(raw, events, preload=preload) 
        
               temp_fname = tmp_path / "test.set" 
        
               # TODO: eeglabio 0.2 warns about invalid events 
        
               if _compare_version(eeglabio.__version__, "==", "0.0.2-1"): 
        
                   ctx = _record_warnings 
        
               else: 
        
                   ctx = nullcontext 
        
               with ctx(): 
        
                   epochs.export(temp_fname) 
        
               epochs.drop_channels([ch for ch in ["epoc", "STI 014"] if ch in epochs.ch_names]) 
        
               epochs_read = read_epochs_eeglab(temp_fname, verbose="error")  # head radius 
        
               assert epochs.ch_names == epochs_read.ch_names 
        
               cart_coords = np.array([d["loc"][:3] for d in epochs.info["chs"]])  # just xyz 
        
               cart_coords_read = np.array([d["loc"][:3] for d in epochs_read.info["chs"]]) 
        
               assert_allclose(cart_coords, cart_coords_read) 
        
               assert_array_equal(epochs.events[:, 0], epochs_read.events[:, 0])  # latency

Specifically right now the test on Line 559 is failing, i.e. the event latencies change across an IO trip.

The culprit is this block of code:

https://github.com/jackz314/eeglabio/blob/cf92efeb92f6ac45f28df50014b4f23ffe622d47/eeglabio/epochs.py#L104-L109

Basically, I think that eeglabio needs to be able to handle 1) the first_samp of a file and 2) the true number of trials before epochs get dropped. AFAICT, the direct path forward is to add a new parameter export_kwargs to epochs.export, that gets threaded to eeglabio.epochs.export_set. Then we can pass a first_samp and n_trials argument, which I am proposing to add over at jackz314/eeglabio#18

Reference issue (if any)

What does this implement/fix?

Additional information

Needs jackz314/eeglabio#18

mne/export/tests/test_export.py

for more information, see https://pre-commit.ci

…b passing again" AND "WIP, FIX: refresh events" Manual revert of commits 5ed2655 a9c163b Taking a second stab at this. We might not need to thread any kwargs to eeglabio from the public API after all...

If https://github.com/jackz314/eeglabio/blob/f496ba45b5db716360e05c24efd89339f7bd2434/eeglabio/epochs.py#L101 is Correct, then MATLAB stores sample number starting at 1 and not at 0

Hand in Hand with jackz314/eeglabio#18 I am not sure if this is the right way to go but at least our test will pass now..

mne/io/eeglab/eeglab.py

scott-huberty · 2025-09-24T00:17:04Z

OK After 3 years, test_export_epochs_eeglab is passing : )

https://github.com/mne-tools/mne-python/actions/runs/17961850120/job/51086635335?pr=13428#step:19:3897

I am going to leave this PR in draft mode as a safeguard because I change the dependencies to point eeglabio to my branch over at https://github.com/jackz314/eeglabio/pull/18/files, which this PR depends on. The bulk of the fix is happening over there.

mne/export/tests/test_export.py

mne/export/_eeglab.py

larsoner · 2025-09-24T20:34:27Z

Okay: https://pypi.org/project/eeglabio/0.1.2/

Should be able to revert the special-branch stuff and continue

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

This reverts commit 51cff19.

larsoner

Other than one reST issue, LGTM +1 for merge! Feel free to commit the fix, mark as ready for review, and mark for merge-when-green @scott-huberty

doc/changes/dev/13428.bugfix.rst

…eeeglab

* 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) ...

…ols#13428) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

FIX, TST, WIP: Try to get test_export_epochs_eeeglab passing again

a9c163b

Needs jackz314/eeglabio#18

scott-huberty requested review from agramfort, dengemann, drammock, mscheltienne and sappelhoff as code owners September 23, 2025 00:09

larsoner reviewed Sep 23, 2025

View reviewed changes

mne/export/tests/test_export.py Outdated Show resolved Hide resolved

scott-huberty marked this pull request as draft September 23, 2025 15:42

scott-huberty and others added 11 commits September 23, 2025 08:44

WIP, FIX: refresh events

5ed2655

DOC: changelog

40b464b

TMP: fix eeglabio install to my fork/branch with fix

1bd1ad6

[pre-commit.ci] auto fixes from pre-commit.com hooks

0ea0345

for more information, see https://pre-commit.ci

workaround to install eeglabio dev branch..

51cff19

DOC, FIX: file name..

3d502a3

Manual revert of "FIX, TST, WIP: Try to get test_export_epochs_eeegla…

ab4cc8d

…b passing again" AND "WIP, FIX: refresh events" Manual revert of commits 5ed2655 a9c163b Taking a second stab at this. We might not need to thread any kwargs to eeglabio from the public API after all...

FIX: Adjust for MATLAB's 1-based indexing of sample number

ac0ece9

If https://github.com/jackz314/eeglabio/blob/f496ba45b5db716360e05c24efd89339f7bd2434/eeglabio/epochs.py#L101 is Correct, then MATLAB stores sample number starting at 1 and not at 0

Take 2: Explicitly pass in epoch indices to eeglabio

2caa388

Hand in Hand with jackz314/eeglabio#18 I am not sure if this is the right way to go but at least our test will pass now..

remove cruft newline

14df67b

remove stale export_kwargs param

38191cb

scott-huberty commented Sep 24, 2025

View reviewed changes

mne/io/eeglab/eeglab.py Show resolved Hide resolved

larsoner reviewed Sep 24, 2025

View reviewed changes

mne/export/tests/test_export.py Show resolved Hide resolved

mne/export/_eeglab.py Outdated Show resolved Hide resolved

scott-huberty and others added 2 commits September 24, 2025 13:42

Incorporate @larsoner suggestions

39e52a3

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

Revert "workaround to install eeglabio dev branch.."

d72b8ef

This reverts commit 51cff19.

larsoner approved these changes Sep 24, 2025

View reviewed changes

doc/changes/dev/13428.bugfix.rst Outdated Show resolved Hide resolved

scott-huberty added 2 commits September 24, 2025 14:03

DOC: update changelog description

80136c5

Merge remote-tracking branch 'upstream/main' into test_export_epocsh_…

2f31621

…eeeglab

scott-huberty marked this pull request as ready for review September 24, 2025 21:06

larsoner enabled auto-merge (squash) September 24, 2025 21:43

larsoner merged commit 3e29187 into mne-tools:main Sep 24, 2025
32 checks passed

scott-huberty deleted the test_export_epocsh_eeeglab branch September 24, 2025 22:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

FIX, TST: Try to get test_export_epochs_eeeglab passing again#13428

FIX, TST: Try to get test_export_epochs_eeeglab passing again#13428
larsoner merged 16 commits intomne-tools:mainfrom
scott-huberty:test_export_epocsh_eeeglab

scott-huberty commented Sep 23, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

scott-huberty commented Sep 24, 2025

Uh oh!

Uh oh!

Uh oh!

larsoner commented Sep 24, 2025

Uh oh!

larsoner left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	def test_export_epochs_eeglab(tmp_path, preload):
	"""Test saving an Epochs instance to EEGLAB's set format."""
	eeglabio = pytest.importorskip("eeglabio")
	raw, events = _get_data()[:2]
	raw.load_data()
	epochs = Epochs(raw, events, preload=preload)
	temp_fname = tmp_path / "test.set"
	# TODO: eeglabio 0.2 warns about invalid events
	if _compare_version(eeglabio.__version__, "==", "0.0.2-1"):
	ctx = _record_warnings
	else:
	ctx = nullcontext
	with ctx():
	epochs.export(temp_fname)
	epochs.drop_channels([ch for ch in ["epoc", "STI 014"] if ch in epochs.ch_names])
	epochs_read = read_epochs_eeglab(temp_fname, verbose="error") # head radius
	assert epochs.ch_names == epochs_read.ch_names
	cart_coords = np.array([d["loc"][:3] for d in epochs.info["chs"]]) # just xyz
	cart_coords_read = np.array([d["loc"][:3] for d in epochs_read.info["chs"]])
	assert_allclose(cart_coords, cart_coords_read)
	assert_array_equal(epochs.events[:, 0], epochs_read.events[:, 0]) # latency

Uh oh!

Conversation

scott-huberty commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference issue (if any)

What does this implement/fix?

Additional information

Uh oh!

Uh oh!

Uh oh!

scott-huberty commented Sep 24, 2025

Uh oh!

Uh oh!

Uh oh!

larsoner commented Sep 24, 2025

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

scott-huberty commented Sep 23, 2025 •

edited

Loading