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

API: Use annotations for read_raw_egi events rather than synthetic channel #12262

Closed
larsoner opened this issue Dec 4, 2023 · 3 comments · Fixed by #12300
Closed

API: Use annotations for read_raw_egi events rather than synthetic channel #12262

larsoner opened this issue Dec 4, 2023 · 3 comments · Fixed by #12300
Assignees
Milestone

Comments

@larsoner
Copy link
Member

larsoner commented Dec 4, 2023

In #4017 (comment) we added RawEGI.event_id which disappears on round-trip. This is due to not knowing which/how many DIN channels will get triggers.

Nowadays we should use Annotations rather than synthesize an event channel I think. Not sure if it's worth keeping the event channel synthesizing around for backward compat or deprecating it. But either way adding the annotations will make it possible for people to use modern code rather than relying on raw.event_id here.

See also discourse for discussion.

@larsoner larsoner added this to the 1.7 milestone Dec 4, 2023
@scott-huberty
Copy link
Contributor

Given that it's been a while now that we've wanted to switch to using mffpy for reading EGI files ( #6937 , #8038 , #11380 ), maybe it would be a good time to implement that, and while doing so, make this transition from synthetic channels to annotations?

@larsoner
Copy link
Member Author

larsoner commented Dec 4, 2023

Perhaps. But I think switching to mffpy is potentially a much bigger undertaking and so far no one has volunteered. And either way we need to go from DIN events (extracted from somewhere) to mne.Annotations, so a first small PR to add DIN-as-annot wouldn't be wasted effort. @scott-huberty if you are interested in learning a bit of EGIMFF the first small PR would be a good place to start :)

@scott-huberty
Copy link
Contributor

I've worked on that module and have used EGI a lot, so go ahead and assign to me!

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 a pull request may close this issue.

2 participants