craddm / eegUtils

An R package for processing and plotting of electroencephalography (EEG) data
https://craddm.github.io/eegUtils/
Other
104 stars 27 forks source link

Trouble using electrode locations from R package eegkit #1

Closed mvuorre closed 7 years ago

mvuorre commented 7 years ago

I am having trouble with topoplot() when using electrode locations provided in the eegkit R package (data(eegcoord, package = "eegkit")). I assume those locations are fairly standard.

Here's the result when I use those locations, with data d:

topoplot(d)

rplot

I have fixed these issues and made topoplot() load these locations by default if locations are not passed in data. Resulting figure is here (I've added a few options e.g. argument for contours):

topoplot(d, rmax = 10, chan_marker = "name", contour=F)

better

The fixes can be found here: https://github.com/mvuorre/eegUtils. I haven't submitted a pull request because I'm not sure if you'd want to use these electrode locations as default (or if the changes are good overall.) Let me know if you'd like me to submit it, though.

Thanks for the great function(s)!

craddm commented 7 years ago

Thanks Matti. There are some default locations already included - it should load them if no locations are passed in the data frame. They're in electrodeLocs, which is loaded via sysdata.rda, so won't show up using data(). Probably need to make sure those join in a case-insensitive manner too...

There's a big difference in scale between the eegkit locations and the default ones I included. As a general solution I need to make it more robust to that, as the code at the moment relies on constants based on the included default locations. I see on your plot the nose has gone missing (guessing it's the black dot just above Cz) which is bound to be for the same reason.

Adding a contour flag is a great idea.

mvuorre commented 7 years ago

I see, the default locations in sysdata probably didn't work in my case because of the case-sesitivity (why are these not standardized across manufacturers etc??).

The nose was indeed missing because of the x-y scales. I've fixed it in a later commit in my branch such that it finds the correct y coordinates from the head circle.

Cheers

craddm commented 7 years ago

I've actually fixed it now in my last couple of commits. Basically I've made it rescale co-ordinates internally. Might cause some issues if people want to annotate the plot afterwards but only if they want to use the original x-y co-ordinates. Also added the option to interpolate only up to the head limits etc. Will add contour flag later. See how you get on with this fix.

mvuorre commented 7 years ago

Not quite working:

> unique(d$electrode)
 [1] "AF3" "AF4" "AF7" "AF8" "AFz" "C1"  "C2"  "C3"  "C4"  "C5"  "C6"  "CP1" "CP2" "CP3" "CP4" "CP5" "CP6" "CPz" "Cz" 
[20] "F1"  "F2"  "F3"  "F4"  "F5"  "F6"  "F7"  "F8"  "FC1" "FC2" "FC3" "FC4" "FC5" "FC6" "FCz" "FP1" "FP2" "FPz" "FT7"
[39] "FT8" "Fz"  "O1"  "O2"  "Oz"  "P1"  "P2"  "P3"  "P4"  "P5"  "P6"  "P7"  "P8"  "PO3" "PO4" "PO7" "PO8" "POZ" "Pz" 
[58] "TP7" "TP8"
> topoplot(d)
Adding standard electrode locations...
Error in qr.default(a, tol = tol) : 
  NA/NaN/Inf in foreign function call (arg 1)
> topoplot(d, skirt = FALSE)
Error in topoplot(avg, skirt = FALSE) : unused argument (skirt = FALSE)

I see this last one fails because skirt is not an argument (although it is in the help page, but this is presumably because you didn't document() yet.

craddm commented 7 years ago

Forgot to update the docs. Changed skirt to interp_limits ("head" or "skirt). Can't replicate the qr.default problem.

mvuorre commented 7 years ago

It seems that the problem was in joining the two tables (default locations and data) when the electrode names had different case. I sent in a PR for the topoplot.R file to fix it.

mvuorre commented 7 years ago

One more thing: would it be reasonable to join the two frames such that it automatically drops electrodes not found in default electrodeLocs (mastoids etc). Cheers

craddm commented 7 years ago

Guess so, although should probably give a warning. (Actually mastoids are in there as M1 and M2 😄).