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

Deprecating events in favor of annotations? #6018

Closed
cbrnr opened this issue Mar 4, 2019 · 23 comments
Closed

Deprecating events in favor of annotations? #6018

cbrnr opened this issue Mar 4, 2019 · 23 comments

Comments

@cbrnr
Copy link
Contributor

cbrnr commented Mar 4, 2019

I was wondering why MNE still needs both events and annotations. We are clearly moving towards annotations as a more flexible container for discrete markers on the file I/O side, but it is difficult for users to understand the differences between the two structures (e.g. the events/annotations tutorial basically says that these structures are identical except for minor implementation differences).

I haven't been able to come up with a single use case that requires the old events array (other than supporting historical code). Therefore, I suggest to deprecate the old events structure and replace everything with annotations, such as mne.find_events (used to parse analog stim channels), which should return an Annotations object. For compatibility with old code, we already have mne.events_from_annotations in case anyone still needs that.

@agramfort
Copy link
Member

agramfort commented Mar 4, 2019 via email

@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 4, 2019

Annotations and events aim to match what you find in different file formats.

You mean both aim to do the same thing, right?

Events is natural if you have stim channels and manipulation of an events array is also trivial. Annotations can be mapped to events and event_id so it allows to construct epochs like we always did.

That's what I'm saying, it seems like we could use annotations everywhere. At least I'd argue for using only annotations when we're talking about what is visible to the user (we can still convert annotations to events if this makes some functions easier to write/maintain - and this already seems to be happening, because what we find in different file formats get written to an Annotations object, which we then can convert to events using mne.events_from_annotations if necessary).

So I take this as a yes from you 😄? To be clear, it seems like this change would mostly affect documentation, where we would highlight annotations as the primary means to store discrete markers (and we could still mention the old events structure, but clarify that end users shouldn't really have to use that).

@agramfort
Copy link
Member

agramfort commented Mar 4, 2019 via email

@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 4, 2019

Are you suggesting to remove events from Epochs construction?

Nope, since this continues to work because we have mne.events_from_annotations. I'm suggesting to remove the events structure from the docs because annotations are better.

@agramfort
Copy link
Member

agramfort commented Mar 4, 2019 via email

@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 4, 2019

I was referring to events/annotations tutorial, which in my opinion creates more confusion because it also mentions events. I think it should focus on annotations.

@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 4, 2019

There are probably more changes involved than I thought, because some viz functions plot events. I mean, all of this could also be done with annotations, but it requires some work. Just to clarify, I didn't want to suggest that we change everything to use annotations, but I wanted to make it easier for users who are currently presented with two options for storing event information...

@agramfort
Copy link
Member

agramfort commented Mar 4, 2019 via email

@massich
Copy link
Contributor

massich commented Mar 4, 2019

There's a newer version https://mne-tools.github.io/dev/auto_tutorials/plot_object_annotations.html

I'm +1 to improve the tutorial. Maybe a table with which files have events and which have annotations would help.
And maybe this events tutorial could be integrated.

@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 4, 2019

Yeah, the thing is that files don't have events or annotations. They store markers in different ways, and we now read them into annotations (whereas previously, we used to synthesize an analog stim channel and convert that to events). I think basically everything can and should be done with annotations now.

@agramfort
Copy link
Member

agramfort commented Mar 4, 2019 via email

@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 4, 2019

How do you get string descriptions needed by annotations from a stim channel?

I don't. But I can convert an analog stim channel to annotations. I don't have to use events.

@dengemann
Copy link
Member

please let’s not touch historical core APIs that still guarantee painless interoperability with MNE-C. Also +1 for improving the docs.

@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 4, 2019

But that's not at all what I'm saying. The old events API can of course stay for internal and historical use, but we should only advocate annotations to users.

@agramfort
Copy link
Member

agramfort commented Mar 4, 2019 via email

@jasmainak
Copy link
Member

I find the events array quite convenient and easier to manipulate. Annotations and events are not the same in my opinion. With numpy arrays, I can search, slice etc. but it's not possible to do these with annotations. It's just a different hammer with different conveniences.

@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 5, 2019

Exactly, it's a different hammer for doing the same thing. Searching and slicing could be added to annotations as well. For me it is confusing to have two things for the same purpose. As we know, there should be one-- and preferably only one --obvious way to do it.

Anyways, I'm fine with improving the docs for now to make it clear that markers loaded from files end up in annotations, and parsed analog stim channels end up in events.

I'd like to hear more opinions though - what do others think? @larsoner @jona-sassenhagen @mmagnuski @wmvanvliet

@jona-sassenhagen
Copy link
Contributor

My use cases agree with @jasmainak. But I agree the current situation is complicated and unfortunate. As always, docs could be improved, and i can imagine a future without events. But I fear we would strand a lot of users if we abandoned events now.

@mmagnuski
Copy link
Member

It is true that both events and annotations in a general sense do the same thing and one could even say that these are different representations of the same thing. However events are more specialised and more directly related to experimental triggers which one uses in epoching and annotations, at least in my mind, are user-generated and need to have some duration - so exactly how they started to be: a way to mark bad periods in the signal. I expect this distinction to be more blurred as we progress (it already is overlapping) until one shiny day event arrays will not be necessary.
For the time being I would focus on improving the docs. So we seem to all agree here. :)

@mmagnuski
Copy link
Member

Currently most scenarios involve reading annotations and than instatiating event from them so I'd say annotations are abstract formless platonic beings that can be carved into the concrete events of interest. ;)

@larsoner
Copy link
Member

larsoner commented Mar 5, 2019

I would say we should stick to updating docs for now. If we can push Annotations to be more and more usable as a replacement for events, then maybe someday we can replace them. However, knowing how much code I already have to mangle events for various paradigms for use with Epochs -- and what a fundamental step it has been in analysis pipelines from the beginning of MNE -- I'm skeptical that the value we would get is worth the pain we would cause by forcing people to replace such things with all Annotations-based processing.

@larsoner
Copy link
Member

larsoner commented Jun 6, 2019

See also discussion that was continued in #6326 (comment)

@larsoner
Copy link
Member

larsoner commented Oct 2, 2023

Based on the discussion above and IRL discussion with @drammock and @agramfort I think we can close this and continue to incrementally improve our docs and uses in examples

@larsoner larsoner closed this as completed Oct 2, 2023
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

No branches or pull requests

8 participants