Back to the main page.
Bug 1877 - ft_filetype will recursively check all subfolders
| Status | CLOSED FIXED |
| Reported | 2012-12-10 14:54:00 +0100 |
| Modified | 2019-08-10 12:03:27 +0200 |
| Product: | FieldTrip |
| Component: | fileio |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Windows |
| Importance: | P3 normal |
| Assigned to: | Bart Gips |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: |
Bart Gips - 2012-12-10 14:54:26 +0100
Line ~635 says: elseif isdir(filename) && any(ft_filetype({ls.name}, 'neuralynx_ds','nowarning',true)) % a downsampled Neuralynx DMA file can be split into three seperate lfp/mua/spike directories % treat them as one combined dataset the any(ft_filetype({ls.name})) is very annoying. If you by accident input a wrong directory (which can happen if you use ft_read_spike(dir)) ft_filetype will check all subdirectories for neuralynx data. I have added a flag that will sillence every warning saying it cannot read the files in the subdirectories in this particular case. However I would also like to add a flag that adds a counter that counts how many times the function has been called recursively so that the function can be forced to stop digging. (i.e. add a persistent variable keeping track of the amount of function calls, provided that it has been called like: any(ft_filetype({ls.name}, 'neuralynx_ds','nowarning',true,'subdirCount',true)) Is this okay?
Robert Oostenveld - 2012-12-10 17:24:21 +0100
After a quick review, my feeling is that this is not a good change. There is no way that any ft_xx high level function would pass the nowarning option, which makes the option irrelevant for most people. If you want to avoid a warning, you would use warning('off', 'ID') in matlab. If you think that FT should not repeat the warning many times, please use warning_once About the code in general: the parsing of default options should all happen at the same place, and not with 10 lines in between So rather than the current 7122 bargip % check whether nowarning has been flagged. 7122 bargip % this is flagged used below: any(ft_filetype({ls.name}, 'neuralynx_ds','nowarning',true)) 7122 bargip % without this flag, ft_filetype would give a warning for every file in the 7122 bargip % directory it cannot determine the filetype of. 7122 bargip nowarning = ft_getopt(varargin,'nowarning',0); 7122 bargip 5629 roboos if isa(filename, 'memmapfile') 1 roboos filename = filename.Filename; 1 roboos end 1 roboos 1 roboos % % get the optional arguments 4437 roboos % checkheader = ft_getopt(varargin, 'checkheader', true); it would come here after the "get the optional arguments statement" A better name of the option would be "warning", which could then assume the value "always", "once", or "never". The specifics of the neuralynx format is better discussed in general at the FT meeting, rather that adding code (=bloat in this case) to fieldtrip. I see a much more elegant solution here. Let's discuss on Wednesday (which means you have to attend once more).
Robert Oostenveld - 2012-12-10 17:28:37 +0100
again some general points: - the nowarning option is not documented - it takes a default value of 0, which should be false - values like 'yes'/'no' should be supported (using the istrue helper function) - please use consistent spacing in the code: 7122 bargip type{i} = ft_filetype(filename{i}, desired,'nowarning',nowarning); should have been 7122 bargip type{i} = ft_filetype(filename{i}, desired, 'nowarning', nowarning); Keeping the code "clean" with this level of detail may sound as a nuisance, but is very important to keep the code readable and thereby maintainable over the next 5 years.
Bart Gips - 2012-12-11 09:41:08 +0100
(In reply to comment #2) Agreed, I'll revert the changes. Thanks for the advice. Making changes to the 'neuralynx directory' filetype detection/structure would be better than the workaround. I'll join the meeting tomorrow.
Bart Gips - 2012-12-12 16:54:46 +0100
Added this subfunction to check for neuralynx_cds %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% % SUBFUNCTION that checks whether the directory is neuralynx_cds %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% function res = filetype_check_neuralynx_cds(filename) res=false; files=dir(filename); dirlist=files([files.isdir]); % 1) check for a subdirectory with extension .lfp, .mua or .spike haslfp = any(filetype_check_extension({dirlist.name}, 'lfp')); hasmua = any(filetype_check_extension({dirlist.name}, 'mua')); hasspike = any(filetype_check_extension({dirlist.name}, 'spike')); % 2) check for each of the subdirs being a neuralynx_ds if haslfp || hasmua || hasspike sel=find(filetype_check_extension({dirlist.name}, 'lfp')+... filetype_check_extension({dirlist.name}, 'mua')+... filetype_check_extension({dirlist.name}, 'spike')); neuralynxdirs=cell(1,length(sel)); for n=1:length(sel) neuralynxdirs{n}=fullfile(filename, dirlist(sel(n)).name); end res=any(ft_filetype(neuralynxdirs, 'neuralynx_ds')); end %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% __________________________ This should be okay. Should we test this on an actual neuralynx_cds dataset? Right now I just made the three directories all containing a mock test.nev file. I'll look it over in the morning for any glaring errors and I'll commmit it then.
Eelke Spaak - 2013-01-23 13:58:32 +0100
While browsing through the NEW bugs, this one caught my eye. It seems it should be assigned to Bart?
Bart Gips - 2013-01-23 14:09:12 +0100
(In reply to comment #5) Whoops, forgot to close it. Fixed since r7161: http://code.google.com/p/fieldtrip/source/detail?r=7161