Back to the main page.
Bug 1471 - providing dataset without full path can lead to unexpected behaviour -> propose warning in ft_filetype
| Status | ASSIGNED |
| Reported | 2012-05-11 11:18:00 +0200 |
| Modified | 2012-05-30 11:54:41 +0200 |
| Product: | FieldTrip |
| Component: | core |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Windows |
| Importance: | P3 normal |
| Assigned to: | Robert Oostenveld |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: |
Roemer van der Meij - 2012-05-11 11:18:33 +0200
I noticed that many people in the Toolkit cd'd to their data directory and supplied the local filename/dirname as input to ft_definetrial/preprocessing. This can lead to unexpected behavior, such as the following example: a low-level read function read_bdf_16 (can't remember exact name) throwing an error of 'not enough input arguments'. I happen to saw that the input dataset was a local filename, the Tookit attendant changed it to a full path, and the issue disappeared. These bugs are very difficult for end-users to track down, and can largely be prevented with a simple warning. I propose to add the following check to the top of ft_filetype.m % check if isempty(strfind(filename,'/')) && isempty(strfind(filename,'\')) warning_once('It appears you specified a local filename/directory, this can lead to unexpected behavior! You should specify to full path including all (sub-)directories.') end
Robert Oostenveld - 2012-05-11 16:27:47 +0200
The disadvantage of having full paths in scripts is that scripts are not portable between computers (desktops and laptops and linux etc). And you also cannot rename a higher level directory without having to edit many scripts. So as such I would not recommend it. If there is an issue with the file not being found, then I suggest that we improve the checking on the presence of the file. Note that ft_filetype is also used to check the filetype of files that do not yet exist (i.e. to determine the output format). It might be more something for the dataset2files function, or at least somewhere around there in ft_checkconfig. That would cause the check to be performed in high-level functions, but not in low-level functions. But to be honest I think that the actual error needs to be tracked down and understood in more detail. The function is probably read_24bit.mex (the m file in in fieldtrip/src) which is used for bdf data.
Roemer van der Meij - 2012-05-11 17:26:05 +0200
The downside of not having full scripts would be that you can only use unique names for every data-file you have, I guess that's matter of preference. Can we be sure that nowhere in the code (including stuff in external) a cd is performed? I was thinking that the thing I ran into was only of many possible errors (I always work with full paths and thought that was pretty common). On the function that was generating the error, the only thing I am still certain about that it started with 'read', and had '16' in the name. Looking at fileio/private, the only one that could cause the error would be read_16bit, which is a mex-file. The guy was working on a laptop, but I can't remember whether it was a linux, mac or windows machine (and didn't ask whether it was a 32 or 64 bits). Looking at the source of the mex, I remember seeing the following error: if (nrhs != 3) mexErrMsgTxt ("Invalid number of input arguments"); Could it be such that when this mex-function get's an empty [], it is not counted as an input argument? After doing a grep on read_16bit: ./fileio/private/read_biosemi_bdf.m: % buf = read_16bit(filename, offset, numwords); ./fileio/private/read_edf.m: buf = read_16bit(filename, offset, numwords); Only the last one is still calling read_16 (biosemi has it uncommented), but I see no issues with the call whatsoever. Maybe the error from read_16bit indicates an issue related to a bad filename? Puzzle puzzle puzzle :)
Roemer van der Meij - 2012-05-16 13:18:22 +0200
*** Bug 1256 has been marked as a duplicate of this bug. ***
Roemer van der Meij - 2012-05-16 13:47:53 +0200
*** Bug 1341 has been marked as a duplicate of this bug. ***
Robert Oostenveld - 2012-05-30 11:52:27 +0200
(In reply to comment #4) after careful deliberation with Roemer on the phone, we determined that the edf/bdf problem is due to the low-level c-code mex implementation not including the full MATLAB search path when determining the actual location of the file, whereas the m-code other functions (such as ft_filetype) do include the MATLAB search path. Consequently the error message in ft_read_header line 92-93 was not triggered, and the error surfaced later in the mex file. The agreed upon solution for mex file readers is that they should be consistent with non-mex implementations, which means that they should allow for the user specifying a string pointing to a file that is not in the pwd but somewhere else on the MATLAB search path. I will craft a test script, based on that the specific solution can be implemented. After that, we'll have to check whether that deals with the complete reported and/or perceived problem.