Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: fix for empty event durations #8384

Merged
merged 11 commits into from Nov 10, 2020

Conversation

mmagnuski
Copy link
Member

@ociepkam - Try if this works for you.

Reference issue

Fixes #8383.

What does this implement/fix?

It seems some eeglab files contain empty durations - these are read as empty errays from matlab files and cause error when trying to set the durations array.

I'll add a test later.

@ociepkam
Copy link

It works. Thank you!

mne/io/eeglab/eeglab.py Outdated Show resolved Hide resolved
@mmagnuski
Copy link
Member Author

@cbrnr Thanks for the comments, I'll return to this tomorrow.

@agramfort
Copy link
Member

@mmagnuski can you add a what's new? thx +1 for MRG

@hoechenberger
Copy link
Member

Can we add a test for this please? :)

@mmagnuski
Copy link
Member Author

@agramfort Sure!
@hoechenberger of course, by "I'll add a test later" I meant later in the time scope of this PR. :)

@hoechenberger
Copy link
Member

Great, looking forward!

@hoechenberger
Copy link
Member

Friendly poke @mmagnuski :)

@mmagnuski
Copy link
Member Author

Thanks for reminding @hoechenberger, I'm on low time budget recently, sorry for delays (but, hey, it's been only 18 days! ;) )
I'll get to this today.

@hoechenberger
Copy link
Member

@mmagnuski Great! And no worries, I can very much relate.

@mmagnuski
Copy link
Member Author

mmagnuski commented Nov 8, 2020

I don't understand why codecov is angry at me... :(
edit: for no apparent reason codecov likes me again

@mmagnuski
Copy link
Member Author

@larsoner - less smoke now and we can see the green CI fields. :)

shutil.copyfile(op.join(base_dir, 'test_raw.fdt'),
empty_dur_fname.replace('.set', '.fdt'))
raw = read_raw_eeglab(input_fname=empty_dur_fname, preload=True)
assert (raw.annotations.duration == 0).all()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't some or all of these been np.nan based on the conditional in eeglab.py and the ev.duration = np.array([], dtype='float') above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the eeglab reader turns np.nan somewhere later into zeros. I can use zeros instead of nans if you think this would be better. But we may also decide to expose the nan durations in annotations at some time in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense!

@larsoner
Copy link
Member

I killed most CIs as we can safely ignore them (I just changed latest.inc -- left CircleCI running just to be safe)

@mmagnuski
Copy link
Member Author

Thanks for the corrections!

@larsoner larsoner merged commit 108e165 into mne-tools:master Nov 10, 2020
0 of 3 checks passed
@larsoner
Copy link
Member

Thanks @mmagnuski !

@mmagnuski
Copy link
Member Author

Thanks @larsoner @cbrnr @agramfort @hoechenberger ! 🚀

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.

Problem with load eeglab file
6 participants