Fix EEG electrode positions in EDF/BDF import. #1541
Conversation
When importing EDF/BDF files, an optional .htps file can be supplied to provide electrode coordinates. This commit fixes the way these are stored in the .info dictionary, so that functions such as make_eeg_layout can use them.
mne/io/edf/edf.py
Outdated
There was a problem hiding this comment.
why commented out? why point extra? if the latter is the correct one the commented line should be deleted
There was a problem hiding this comment.
fit_sphere_to_headshape expects the points to be of kind EXTRA:
# get head digization points, excluding some frontal points (nose etc.)
hsp = [p['r'] for p in info['dig'] if p['kind'] == FIFF.FIFFV_POINT_EXTRA
and not (p['r'][2] < 0 and p['r'][1] > 0)]
Whether or not this makes any sense I don't know.
mne/io/edf/edf.py
Outdated
There was a problem hiding this comment.
@t3on @kazemakase looks good to you? any thing we should be careful about?
There was a problem hiding this comment.
I don't understand why this should relabeled. I labeled these according to the conventions listed in the mne-manual (pdf p.272): Digitization point which is a cardinal landmark aka. fiducial point.
There was a problem hiding this comment.
then we should change fit_sphere_to_headshape to look for points of type CARDINAL, at the moment it looks for points of type EXTRA. There is probably a reason for this: maybe some devices export the head digitization points as EXTRA?
There was a problem hiding this comment.
Sorry, I don't know what point_dict is used for. Do these describe what the electrode positions are to MNE? In that case I don't think the positions are actually landmarks, are they?
There was a problem hiding this comment.
fit_sphere_to_headshape should indeed look for 'landmark' points, which are of type CARDINAL. Only if these are not present it should look for points of type EXTRA (to retain backwards compatibility). I've reversed the changes in the EDF reader and fixed fit_sphere_to_headshape instead.
Currently, fit_sphere_to_headshape only takes digitazation points of type 'extra' into account. It should first look for cardinal landmark points and only if these are not found look for points of type 'extra'.
|
The fitting should probably always use cardinal and extra points. For example, our data has both the 3 cardinal landmarks, but also 100+ "extra" digitization points. These changes will make it so that our data are fit using only the 3 cardinal landmarks instead of the 100+ (!), which is quite bad from a fitting perspective. There shouldn't be any problem with adding the cardinal points in with the extra points to do the head fitting, so I'm +1 for just adding the cardinal points to the set of points it considers when fitting. |
|
can you rebase to make travis happy? |
|
forget it. Works for me. Let's see if build bot is happy. |
|
merged by rebase. |
7 participants
When importing EDF/BDF files, an optional .htps file can be supplied to
provide electrode coordinates. This commit fixes the way these are
stored in the .info dictionary, so that functions such as
make_eeg_layout can use them.