Skip to content

Conversation

@massich
Copy link
Contributor

@massich massich commented Dec 17, 2018

This PR adds a parameter to events_from_annotation so that if this parameter
None, events correspond to the annotation onsets (as was happening until nos).
But if not, events_from_annotations returns as many events as they fit within
the annotation duration spaced according to this new parameter (right now is
called chunk_duraiton but we should discuss it before merging).

This PR needs to go in before #5718


Things that are not in this PR (it will be done in subsequent PRs):

  • allow slicing of Annotations
  • update the documentation of events and annotations

@codecov
Copy link

codecov bot commented Dec 17, 2018

Codecov Report

Merging #5795 into master will decrease coverage by 0.11%.
The diff coverage is 20.83%.

@@            Coverage Diff             @@
##           master    #5795      +/-   ##
==========================================
- Coverage   88.53%   88.41%   -0.12%     
==========================================
  Files         369      369              
  Lines       68955    69041      +86     
  Branches    11614    11623       +9     
==========================================
- Hits        61048    61046       -2     
- Misses       5048     5131      +83     
- Partials     2859     2864       +5

@massich massich changed the title [WIP] Get multiple spaced events from single annotation [MRG] Get multiple spaced events from single annotation Dec 17, 2018

logger.info('Used Annotations descriptions: %s' %
(list(event_id_.keys()),))
for onset, duration, description in iterator:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to add slicing of the annotations so that this can be:
for onset, duration, description in raw.annotations[event_sel]:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but this can be done afterwards

Copy link
Member

Choose a reason for hiding this comment

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

Yes a __getitem__ that returns a copy and a __iter__ method (or whatever the correct Py3k one is named) would be good at some point

@massich
Copy link
Contributor Author

massich commented Dec 17, 2018

@Slasnista you should be able to get the psg events just by doing the following:

raw = mne.io.read_raw_edf(psg_fname, stim_channel=False)
annotations = mne.read_annotations(hyp_fname)
desc2int = {'Sleep stage W': 5,
             'Sleep stage 1': 3,
             'Sleep stage 2': 2,
             'Sleep stage 3': 1,
             'Sleep stage 4': 1,
             'Sleep stage R': 4}
events, event_id = mne.events_from_annotations(raw, event_id=desc2int, chunk_duration=30)

@massich
Copy link
Contributor Author

massich commented Dec 17, 2018

I've reverted the last commits where I refactored the code to be simpler because it was messing up with EDF and to fix it would complicate the PR. I'll get back to it once EDF #5741 is in and find_edf_events is out.

@agramfort
Copy link
Member

@massich you have some pep8 errors.

@agramfort
Copy link
Member

@massich you need to rebase

@massich
Copy link
Contributor Author

massich commented Dec 19, 2018

@larsoner can you merge?

@massich
Copy link
Contributor Author

massich commented Dec 19, 2018

After a talk IRL with @agramfort we decided to set back regexp to None instead of '.*'. And revoked the possibility of warning/raising to avoid the parameter at the API.

I see the value of on_missing (that was not the case for the None) but I agree that havning on_missing, verbose .. it keeps polluting the API. But if someone things that would rather have warning instead of error I don't mind revert 71623e8 ( and add a good first issue for someone to factor the code).

@massich
Copy link
Contributor Author

massich commented Dec 19, 2018 via email

Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

+1 for MRG when CIs are green

@larsoner larsoner merged commit e3ef29e into mne-tools:master Dec 19, 2018
@massich massich deleted the spaced_events branch December 20, 2018 10:49
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.

3 participants