Back to the main page.
Bug 3367 - ft_datatype qualifies a mesh structure as 'source+mesh'
Status | CLOSED INVALID |
Reported | 2017-11-04 07:29:00 +0100 |
Modified | 2019-08-10 12:41:47 +0200 |
Product: | FieldTrip |
Component: | core |
Version: | unspecified |
Hardware: | PC |
Operating System: | Mac OS |
Importance: | P5 normal |
Assigned to: | Arjen Stolk |
URL: | |
Tags: | |
Depends on: | |
Blocks: | |
See also: |
Arjen Stolk - 2017-11-04 07:29:34 +0100
Just wondering whether this is desired behavior, as it implies we'd have to re-organize a few switch case situations, e.g. in ft_electrodeplacement*. mesh = ft_read_headshape('freesurfer/surf/lh.pial') mesh = struct with fields: pos: [117024×3 double] tri: [234044×3 double] sulc: [117024×1 double] curv: [117024×1 double] area: [117024×1 double] thickness: [117024×1 double] unit: 'mm' ft_datatype(varargin{1}) ans = 'source+mesh' Fast-forwarding to line 46 in ft_datatype: issource = (isfield(data, 'pos') || isfield(data, 'pnt')) && isstruct(data) && numel(data)==1; % pnt is deprecated, this does not apply to a mesh array ismesh = (isfield(data, 'pos') || isfield(data, 'pnt')) && (isfield(data, 'tri') || isfield(data, 'tet') || isfield(data, 'hex')); % pnt is deprecated > there's a pos field and isstruct(data) returns true, hence ft_datatype thinks it's source, besides the actual mesh. *at line 127 switch ft_datatype(varargin{1}) case 'volume' cfg.method = 'volume'; case 'mesh' cfg.method = 'headshape'; end > there's currently no 'case source+mesh'
Jan-Mathijs Schoffelen - 2017-11-14 10:05:34 +0100
I think that the source+mesh is to be expected in this case, because it contains vectors of numeric data, corresponding with the number of positions. I am not sure what the thought is behind returning 'headshape' in case one asks for a 'mesh' in ft_datatype. Perhaps Simon can comment? Also, is there something that does not work for you, or did you just want to have it mentioned here?
Robert Oostenveld - 2017-11-14 10:06:17 +0100
(In reply to Jan-Mathijs Schoffelen from comment #1) FYI, I just got a bugzilla server email for this...
Jan-Mathijs Schoffelen - 2017-11-14 10:07:22 +0100
Thanks for letting us know. I explicitly added you to the CC-list, so that at least still works :o).
Robert Oostenveld - 2017-11-14 10:44:16 +0100
there is a general issue here: some datatypes are well defined (with a dimord and all) and others are not so well defined. In general the outputs of ft_highlevelfunction(cfg, ...) are well defined, whereas the outputs of ft_read_lowlevel(filename) are not. I would also say that "well defined" implies that there should be a fieldtrip/utilities/ft_dadtatype_xxx for that datatype. The source structure is well defined (an old one, and a new one), whereas headships, meshes, etc are still not well defined.
Arjen Stolk - 2017-11-14 17:26:13 +0100
(In reply to Jan-Mathijs Schoffelen from comment #1) I just wondered. ft_electrodeplacement will fail though when you give it a mesh as input and don't specify cfg.method = 'headshape'. But the headshape-based functionality hasn't been really fully developed anyway (I might do this in near future). Will just leave it with this notification then, just to have made you aware of it.
Robert Oostenveld - 2017-11-14 17:44:47 +0100
(In reply to Arjen Stolk from comment #5) The headshape-based method is mainly the work of Simon (who is on the CC). If there is anything that needs to be improved or fixed, please clarify it to him and try to make it SMART (https://en.wikipedia.org/wiki/SMART_criteria).
Arjen Stolk - 2017-11-14 17:58:54 +0100
(In reply to Robert Oostenveld from comment #6) Well, part of the work that needs to be done is to figure out what we want it to do. I'm thinking it should be able to do the same as the volume-based functionality of ft_electrodeplacement, as in that it should allow reading in electrode labels and spitting out an elec structure. The main difference should then be that the user can mark electrode locations on the surface (e.g. on the basis of intraop photos) instead of localizing within a volume (typically a CT). The basics in terms of selecting a point on the surface seem to be there. Just there's not much more beyond a single click right now. I might take this project up to train a student on fieldtrip coding (Richard Jimenez, also in the Knight lab), if Simon is okay with that.
Simon - 2017-11-15 10:20:07 +0100
(In reply to Arjen Stolk from comment #7) Hey Arjen, As already said by Robert there are some datatypes well defined. Especially the data structure of mesh is similar to a source data structure. (Not yet easily distinguishable). Anyway if you want to use it as a student project, please feel free to improve ft_electrodeplacement.
Robert Oostenveld - 2017-11-15 13:06:16 +0100
(In reply to Simon from comment #8) @Arjen, have a look at http://www.fieldtriptoolbox.org/tutorial/electrode to see how Simon intends it to be used at the moment.
Arjen Stolk - 2017-11-15 17:37:28 +0100
(In reply to Robert Oostenveld from comment #9) Hey both, That looks to be a nice project. Per the implementation in ft_electrodeplacement, I wonder whether you'd find it a useful feature to have the same list box with electrode labels in the figure as we have for localizing electrodes in volumes (see electrode placement figure at http://www.fieldtriptoolbox.org/tutorial/human_ecog). In short, the user would assign electrode labels directly to the current crosshair positions (instead of afterwards), move the crosshair to an already localized electrode by clicking on its label in the listbox, or remove an electroce location altogether by double clicking its label in the listbox. The labels in the listbox can be prespecified by giving electrode labels from some EEGcap montage file information / header file information (hdr.label) to cfg.channel. Another advantage is that the list functions as a to do list for the user. The only obstacle I see is space for this listbox, but we could probably figure that out.
Simon - 2017-11-16 09:36:55 +0100
@Arjen I really like your idea Arjen! Especially the part with the to do list is really handy.
Arjen Stolk - 2017-11-23 05:19:59 +0100
Thanks, Simon. Richard and I made a start today taking up this project and you will hear back from us in the form of a PR.
Robert Oostenveld - 2019-08-10 12:35:34 +0200
This closes a whole series of bugs that have been resolved (either FIXED/WONTFIX/INVALID) for quite some time. If you disagree, please file a new issue on https://github.com/fieldtrip/fieldtrip/issues.