[#6248] fileparts() in WFU_PickAtlas_3.0.3

No commits have been made.

Please log in

State: more information
Closed
Date:
2011-11-10 10:57
Priority: more information
3
Submitted By:
Dimitri Papadopoulos (dpo)
Assigned To: more information
Benjamin Wagner (bwagner)
Hardware: 
None
Operating System: 
None
Component: 
None
Version: 
v3.0
Severity: 
None
Resolution: 
Invalid
SPM Version: 
SPM8
Summary: more information
fileparts() in WFU_PickAtlas_3.0.3

Detailed description

Starting With Matlab R2011a calling fileparts() with four output arguments is no longer supported and has been removed:
http://www.mathworks.fr/help/releases/R2011a/techdoc/ref/fileparts.html
Compare to the same documentation item for Matlab R2010a :
http://www.mathworks.fr/help/releases/R2010a/techdoc/ref/fileparts.html

It looks like almost all calls to fileparts() with four output arguments have been replaced by calls to spm_fileparts() after applying WFU_PickAtlas_3.0.3_Update_for_Matlab_2011b.tgz. Somme occurrences seem to have been left behind.

$ grep fileparts */*/*.m | grep -e '\[[A-Za-z0-9 ]*,[A-Za-z0-9 ]*,[A-Za-z0-9 ]*,[A-Za-z0-9 ]*\]' | grep -v spm_fileparts
toolbox/wfu_pickatlas/wfu_extract_labels.m: [path,file,ext,ver] = fileparts(listname);
toolbox/wfu_pickatlas/wfu_pickatlas.m: [pathstr,name,ext,versn] = fileparts(outfilename);
toolbox/wfu_results/wfu_roi_table.m: [path,name,ext,v] = fileparts(flist(1,:));
toolbox/wfu_results/wfu_roi_table.m: [path,name,ext,v] = fileparts(flist(1,:));
toolbox/wfu_results/wfu_roi_table.m: [path,name,ext,v] = fileparts(deblank(P_list(i,:)));
toolbox/wfu_results/wfu_roi_table.m: [path,name,ext,v] = fileparts(deblank(P_list(i,:)));
toolbox/wfu_tbx_common/wfu_ROI.m:[pth,nm,xt,vr] = fileparts(deblank(PI));
$

Note that I haven't seen any runtime warning while using WFU_PickAtlas. I have merely been auditing the Matlab *.m files in our different SPM versions to prepare an upgrade from Matlab R2011a to R2011b.

Response

Message

Date: 2011-11-10 14:25
Sender: Benjamin Wagner

wfu_extract_labels is not called by the PickAtlas program.  It appears to be cruft.  Part of the consistancy v/s effort problem.  This routine I purposely did not modify so that if it ever is used I will hear about it.


Date: 2011-11-10 14:06
Sender: Dimitri Papadopoulos

The fact that I don't see runtime warnings does not mean I have run the software. I just meant to say that the bug report has been triggerred by a code review, not by actually testing the software (yet).

Are you certain all the remaining occurrences of fileparts() are in try/catch statements? For example I don't see any try/catch in wfu_extract_labels.m but then maybe the try/catch is in some other file which in turn calls wfu_extract_labels.m.

%-------------------------------
%write labels
%-------------------------------
if nargin < 3
     [path,file,ext,ver] = fileparts(listname);
     outname = fullfile(path,[file '_labels.txt']);   
end
[fid, message] = fopen(outname, 'wt');
There are many ways of working around the fileparts() change in Matlab and at least two of them seem to be used in WFU_PickAtlas and specifically in WFU_PickAtlas_3.0.3_Update_for_Matlab_2011b.tgz:
  • changing fileparts() to spm_fileparts(),
  • enclosing fileparts() in a try/catch statements.
For the sake of consistency, it would probably be better to stick to a single workaround unless there are good reasons.

Anyway, this is by no way an urgent matter and can certainly wait for version 3.0.4.


Date: 2011-11-10 13:40
Sender: Benjamin Wagner

Hello Dimitri,
  I must have accidently deleted part of my response.  The response was supposed to be:

Thank you for your diligence with code review.  However, this is a non-issue (as you've noted "I haven't seen any runtime warning").  These fileparts are in try/catch statements.  The next formal release will contain a better fix.

I'm also confused.  You've not seen runtime errors (implies using software) and the statement "I haven't run the software!" conflict.  I will allay that concern with I do use the pickatlas under 2011b without problems. However, if you do have a problem, please let me know.

My appologies for the truncated response.
Ben


Date: 2011-11-10 13:24
Sender: Dimitri Papadopoulos

Why is this an "non-issue"?

I haven't run the software! Therefore the lack of runtime warnings is not enough to decide it's a non-issue. Of course there might be other reasons why it's a non-issue. Would you mind sharing them?


Date: 2011-11-10 13:18
Sender: Benjamin Wagner

Hello Dimitri,
  Thank you for your diligence with code review.  However, this is a non-issue (as you've noted "I haven't seen any runtime warning").

  Please continue to inform us of any problems/concerns/bugs that you may have.

Best,
Ben

Attached Files:

Name Download
No Files Currently Attached

Changes:

Field Old Value Date By
New Message2011-11-10 14:25bwagner
New Message2011-11-10 14:06dpo
New Message2011-11-10 13:40bwagner
New Message2011-11-10 13:24dpo
ResolutionNone2011-11-10 13:18bwagner
New Message2011-11-10 13:18bwagner
assigned_tonone2011-11-10 13:18bwagner
status_idOpen2011-11-10 13:18bwagner