Skip to content

Fix inferring fiducials from EEGLAB #12165

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.

Sign up for GitHub

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

Jump to bottom
Merged
larsoner merged 4 commits into from
Nov 3, 2023
Merged

Fix inferring fiducials from EEGLAB #12165

larsoner merged 4 commits into from
Nov 3, 2023

Conversation

@cbrnr
Copy link
Contributor

@cbrnr cbrnr commented Nov 3, 2023 •
edited
Loading

Fixes #11641 (I can successfully load the files linked in this issue). However, he code is likely still dysfunctional, because we are currently searching for labels "Nasion", "Right periauricular point", and "Left periauricular point" (case sensitive). I am not sure if these names are even standardized, but "periauricular" seems to be a typo on our end, because I have only ever heard the term "preauricular".

But unless we know more about how fiducials are stored in EEGLAB files, we are stuck with what we have I guess.

@cbrnr
Copy link
Contributor Author

cbrnr commented Nov 3, 2023

Hmmm. Why does Sphinx not find mne.io.read_epochs_eeglab()?

larsoner
larsoner reviewed Nov 3, 2023
View reviewed changes
@larsoner
Copy link
Member

larsoner commented Nov 3, 2023

but "periauricular" seems to be a typo on our end, because I have only ever heard the term "preauricular".

I've seen both -- periauricular shows up in the BIDS docs for example https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/02-magnetoencephalography.html

larsoner
larsoner reviewed Nov 3, 2023
View reviewed changes
@larsoner larsoner enabled auto-merge (squash) November 3, 2023 13:37
@cbrnr
Copy link
Contributor Author

cbrnr commented Nov 3, 2023

I don't think periauricular point is correct though. Peri = around, so this refers to the area around the ear, not a specific point. If we're talking about the fiducial point, I think only preauricular (pre = in front of) is correct.

@larsoner larsoner merged commit 3bc18ff into mne-tools:main Nov 3, 2023
@cbrnr cbrnr deleted the fix-eeglab-import branch November 3, 2023 15:03
larsoner added a commit to larsoner/mne-python that referenced this pull request Nov 3, 2023
@larsoner
Merge remote-tracking branch 'upstream/main' into kernel
3611ef8 
* 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)
  ...
@hoechenberger
Copy link
Member

hoechenberger commented Nov 3, 2023 •
edited
Loading

I've seen both -- periauricular shows up in the BIDS docs for example https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/02-magnetoencephalography.html

The BIDS specs contain both terms, even on the same page … 🫣 Clearly either these must be interchangeable or (probably most likely) people mis-type / mis-read these, as they're easy to confuse. In either case, it's always RPA and LPA that's meant.

larsoner added a commit to JD-Zhu/mne-python that referenced this pull request Nov 3, 2023
@larsoner
Merge remote-tracking branch 'upstream/main' into jdz
cf08258 
* 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)
  ...
larsoner added a commit to pablomainar/mne-python that referenced this pull request Nov 8, 2023
@larsoner
Merge remote-tracking branch 'upstream/main' into fix
31c299b 
* upstream/main:
  BUG: Fix bug with spectrum warning (mne-tools#12186)
  Add argument splash to disable splash-screen from Qt-browser (mne-tools#12185)
  BUG: Fix bug with logging and n_jobs>1 (mne-tools#12154)
  Use gray logo (works in light and dark modes) (mne-tools#12184)
  Tweak logo for dark mode (mne-tools#12176)
  ENH: Improve Covariance.__repr__ (mne-tools#12181)
  ENH: Enable sensor-specific OPM coregistration in mne coreg (mne-tools#11405)
  Tweak README.rst (mne-tools#12166)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#12177)
  MAINT: Add branch coverage (mne-tools#12174)
  OpenSSF (mne-tools#12175)
  fix docstring in 60_sleep.py (mne-tools#12171)
  FIX: skip empty lines in read_raw_eyelink (mne-tools#12172)
  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)
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
@cbrnr @snwnde
Fix inferring fiducials from EEGLAB (mne-tools#12165)
cbdfdab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@larsoner larsoner larsoner approved these changes

@drammock drammock Awaiting requested review from drammock drammock is a code owner

@agramfort agramfort Awaiting requested review from agramfort agramfort is a code owner

@dengemann dengemann Awaiting requested review from dengemann dengemann is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

can’t import my set file using read_epochs_eeglab()

3 participants

@cbrnr @larsoner @hoechenberger