[BUG] Drop annotations with NaN onset in EEGLAB raw files

[withmywoessner]

Reference issue

Fixes #12425.

What does this implement/fix?

Fix bug where MNE cannot handle EEGLAB events with NaN annotations

Additional information

Rationale for decision from EEGLAB documentation:

https://eeglab.org/tutorials/04_Import/Importing_Event_Epoch_Info.html

[withmywoessner]

@cbrnr Should be fixed now! Should I add that data to the testing dataset?

[cbrnr]

I would not add a new dataset, but can you add a test with some artificially generated markers that demonstrate the problem (and verify the fix)?

[withmywoessner]

I would not add a new dataset, but can you add a test with some artificially generated markers that demonstrate the problem (and verify the fix)?

Would I have to make a temp file @cbrnr? Because from my understanding, I would have to read a MATLAB .set file then edit the annotations. Then I would need to read the file again to check to see if it drops the bad annotations. Can you think of another way?

EDIT: I think I can just import _read_annotations_eeglab directly

EDIT2: I looked and _read_annotations has some other functions that it is reliant on so I don't think I can just use that to read the events I get from io.loadmat(raw_fname_mat,...)

EDIT3: Yeah, since MATLAB files are stored as a binary and the _read_annotations expects a MATLAB file. I think I would have to read the MATLAB file, then edit the annotations, then re-export it as a binary.

[cbrnr]

Hm, yes, because the fix is in the reader it's probably easiest if you add a small(!) test file then.

[larsoner]

EDIT3: Yeah, since MATLAB files are stored as a binary and the _read_annotations expects a MATLAB file. I think I would have to read the MATLAB file, then edit the annotations, then re-export it as a binary.

Can you try this and see if it's easy? If it's old format then scipy.io functions can be used. If it's new format then h5py could perhaps be used directly just to change one or two entries and save to a tmpdir?

[withmywoessner]

I can try @larsoner should I just use the tempfilelibrary?

[larsoner]

No in tests typically you use the pytest test fixture def test_something(tmp_path): and write for example in the test with open(tmp_path / "my_file.txt', 'w'): or similar.

[withmywoessner]

Should be good now @larsoner and @cbrnr. Thanks!

[withmywoessner]

There is an error related to testing. Not sure what the problem is. When I ran it locally it passed:

[larsoner]

That's a weird error -- I put a little fix in #12491 so ignore it for now and we'll merge that quickly and merge main into your branch afterward to make sure everything is green

[larsoner]

Just a couple of minor things, committed and marking for merge-when-green, thanks @withmywoessner !

[larsoner]

Pip-pre is unrelated, merging!