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

Allow automated metadata generation to be bounded by "row events" instead of explicit time windows #12118

Merged
merged 9 commits into from
Oct 31, 2023

Conversation

hoechenberger
Copy link
Member

@hoechenberger hoechenberger commented Oct 18, 2023

This PR makes it possible to generate metadata (via mne.epochs.make_metadata()) based on time windows of varying duration, bounded by user-specified events. Specifically, users can now pass tmin=None and tmax=None. Then, all events occuring between the time-locked events ("row events") will be added to one row of the metadata.

Use case:
I have EEG data where each trial starts with a visual cue. The experiment is self-paced, hence trial durations vary greatly within and between participants. Specifying the same rigid time window for metadata generation for all trials by passing tmin and tmax as a float (time relative to the time-locked event, in seconds) didn't yield satifisfactory results. A much more natural approach for me was to simply add all events between two cues to the metadata.

mne/epochs.py Outdated Show resolved Hide resolved
"""Test that make_metadata works."""
pytest.importorskip("pandas")
raw, all_events, _ = _get_data()
tmin, tmax = -0.5, 1.5
sfreq = raw.info["sfreq"]
kwargs = dict(
events=all_events,
Copy link
Member

Choose a reason for hiding this comment

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

It would be great in principle to add this to an example or tutorial, but maybe we don't have an appropriate dataset (?).

In lieu of (or in addition to) that, could you add a test that shows that this does what it's supposed to do? Like create a RawArray and events that have uneven spacing, e.g., the second twice as long as the first and third, and the fourth four times as long or whatever. Then make sure that the metadata reflects... whatever it's supposed to reflect. And that creating Epochs using those events "does the right thing" -- which isn't immediately clear to me without seeing such an example in tests or in our docs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, both the code and the tests have gotten extremely convoluted and it's difficult for me to tell what's actually happening just by looking at the code. Should probably be refactored sometime in the not-to-distant future. I will try to add a test like you proposed.

@hoechenberger
Copy link
Member Author

@larsoner I've added a test case with a slightly verbose "setup" where I first define the events in a very user-readable way, convert them to annotations, and only then create events from them. Made it easier for me to follow what I'm actually testing, and inspect intermediate results in a a human-readable way.

@hoechenberger
Copy link
Member Author

Failing tests seem unrelated.

@hoechenberger
Copy link
Member Author

@larsoner This seems to be working, I've been using it with a real-world dataset successfully and so far haven't encountered any obvious problems

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Thanks, the added test did help!

@larsoner larsoner merged commit 60db738 into mne-tools:main Oct 31, 2023
28 checks passed
@hoechenberger hoechenberger deleted the metadata branch October 31, 2023 18:24
@hoechenberger
Copy link
Member Author

Thanks for reviewing!

larsoner added a commit to larsoner/mne-python that referenced this pull request Nov 3, 2023
* upstream/main: (35 commits)
  [DOC] Add documentation for setting montage order (mne-tools#12160)
  Fix inferring fiducials from EEGLAB (mne-tools#12165)
  Try to fix ICA Report (mne-tools#12167)
  BUG: Fix bug with Report.add_ica component number (mne-tools#12156)
  MAINT: Add rstcheck to CIs and pre-commit (mne-tools#12163)
  DOC: fix sphinx style typos (mne-tools#12161)
  MAINT: Fix linkcheck (mne-tools#12162)
  ENH: Add multiple label support to source_band_induced_power, source_induced_power (mne-tools#12026)
  Allow automated metadata generation to be bounded by "row events" instead of explicit time windows (mne-tools#12118)
  ENH: Collapse only in doc gen (mne-tools#12145)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#12155)
  BUG: Fix bug with interior points not showing (mne-tools#12148)
  ENH: Warn about versions in sys_info (mne-tools#12146)
  Fix in conftest.py (mne-tools#12150)
  ENH: set color for bad channel with spatial_colors=True (mne-tools#12142)
  DOC: Better documentation of realign_raw (mne-tools#12135)
  Add mne-icalabel wildcard (mne-tools#12143)
  Remove LGTM.com configuration file (mne-tools#12139)
  DOC: Fix typo found by codespell (mne-tools#12140)
  DOC: Document governance updates (mne-tools#12133)
  ...
larsoner added a commit to JD-Zhu/mne-python that referenced this pull request Nov 3, 2023
* upstream/main: (26 commits)
  FIX: Fix bug with coreg scalars (mne-tools#12164)
  Changed casting rule in np.clip to allow reading of raw GDF files (mne-tools#12168)
  [DOC] Add documentation for setting montage order (mne-tools#12160)
  Fix inferring fiducials from EEGLAB (mne-tools#12165)
  Try to fix ICA Report (mne-tools#12167)
  BUG: Fix bug with Report.add_ica component number (mne-tools#12156)
  MAINT: Add rstcheck to CIs and pre-commit (mne-tools#12163)
  DOC: fix sphinx style typos (mne-tools#12161)
  MAINT: Fix linkcheck (mne-tools#12162)
  ENH: Add multiple label support to source_band_induced_power, source_induced_power (mne-tools#12026)
  Allow automated metadata generation to be bounded by "row events" instead of explicit time windows (mne-tools#12118)
  ENH: Collapse only in doc gen (mne-tools#12145)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#12155)
  BUG: Fix bug with interior points not showing (mne-tools#12148)
  ENH: Warn about versions in sys_info (mne-tools#12146)
  Fix in conftest.py (mne-tools#12150)
  ENH: set color for bad channel with spatial_colors=True (mne-tools#12142)
  DOC: Better documentation of realign_raw (mne-tools#12135)
  Add mne-icalabel wildcard (mne-tools#12143)
  Remove LGTM.com configuration file (mne-tools#12139)
  ...
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
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.

2 participants