Back to the main page.
Bug 591 - ft_neighbourselection should not be called by other FT functions
| Status | CLOSED FIXED |
| Reported | 2011-04-20 13:21:00 +0200 |
| Modified | 2011-07-29 10:34:16 +0200 |
| Product: | FieldTrip |
| Component: | core |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Windows |
| Importance: | P1 normal |
| Assigned to: | Jörn M. Horschig |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: |
Jan-Mathijs Schoffelen - 2011-04-20 13:21:35 +0200
users should call it like cfg.neighbours = ft_neighbourselection(...) and they should be encouraged to do cft_neighbourplot(cfg) todo: grep ft_neighbourselection
Jan-Mathijs Schoffelen - 2011-04-20 13:22:29 +0200
also convert cell-array into struct-arran and implement fixneighbours
Johanna - 2011-05-11 15:42:40 +0200
note to self: check all other bugs pertaining to ft_neighbourselection
Johanna - 2011-05-16 12:21:38 +0200
Right now ft_neighbourplot allows (1) cfg.neighbours, or (2) neighbours to be a third input, or (3) call ft_neighbourselection if not either (1) or (2). Fixing this bug will remove option (3), but I'm wondering whether option (2) makes sense? It seems unconventional for FT code/structure. I would rather require users to put it in cfg. For ft_scalpcurrentdensity.m, cfg.neighbours should now be made a required input if cfg.method ='hjorth'. This is straightforward. However, why does ft_neighbourselection get called if isfield(cfg,'layout') when computing the 'elec' structure in line 122? It seems unnecessary/unused.
Jörn M. Horschig - 2011-07-08 11:12:10 +0200
For sclapcurrentdensity, I just removed neighbourselection if cfg.layout (it looks like a copy/paste error), and added if strcmp(cfg.method, 'hjorth'), cfg = ft_checkconfig(cfg, 'required', {'neighbours'}); end Now, I will start look into these cases, where ft_neighbourselection is not called explicitly, but a neighbourstructure is constructed anyway (paknar gradient, repairchannel, and what else might be there)
Jörn M. Horschig - 2011-07-08 11:12:52 +0200
who does not know paknar gradient, the far superior version of planar gradient
Jörn M. Horschig - 2011-07-11 13:09:03 +0200
*** Bug 662 has been marked as a duplicate of this bug. ***
Jörn M. Horschig - 2011-07-12 12:14:54 +0200
all should be changed - awaiting complaints now ;) (In reply to comment #1) > also convert cell-array into struct-arran and implement fixneighbours just to be sure that I understand this correctly: now it is a cell-array of structs, but it should be a struct-array of cells? sthg like >> cfg.neighbours ans = label = {100x1 cell} neighblabel = {100x1 cell} and the only reason do this is because of readability? Furthermore, only high-level functions should then call fixneighbours and low-level functions should assume cfg.neighbours to be a struct-array, right?
Jan-Mathijs Schoffelen - 2011-07-12 12:39:49 +0200
all should be changed - awaiting complaints now ;) Fingers crossed. (In reply to comment #1) > also convert cell-array into struct-arran and implement fixneighbours just to be sure that I understand this correctly: now it is a cell-array of structs, but it should be a struct-array of cells? sthg like >> cfg.neighbours ans = label = {100x1 cell} neighblabel = {100x1 cell} and the only reason do this is because of readability? Well, it's also easier to handle (in terms of indexing). Struct-array should look sth like: cfg.neighbours(1) ans = label = {'bla'} neighblabel = {'a' 'b' 'c'} Furthermore, only high-level functions should then call fixneighbours and low-level functions should assume cfg.neighbours to be a struct-array, right? yep.
Jörn M. Horschig - 2011-07-12 16:39:37 +0200
I'll commit this tomorrow, so that I got some time to trace bugs before the final release ;)