bugGNU Octave - Bugs: bug #37333, textscan called with empty...

 
 

bug #37333: textscan called with empty whitespace

Submitter:  None
Submitted:  Wed 12 Sep 2012 11:47:19 AM UTC
   
 
Category:  Libraries Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  philipnienhuis
Originator Name:  Andy Register Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 3.6.2
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 01 Jan 2013 09:08:19 PM UTC, comment #12: 

Andy: sorry for the long delay.

At last I found time to add your tests to the repo (all of them, edited for style), attributed to you. See:
http://hg.savannah.gnu.org/hgweb/octave/rev/6251fa48d28b

Philip Nienhuis <philipnienhuis>
Group Member
Fri 14 Sep 2012 09:51:15 PM UTC, comment #11: 

I went back and looked at the submitted file.  The code is correct in that it uses a ' ' in the textscan whitespace argument.  It is just the comment that is wrong.  So the original code with the line


c = textscan(string, '%s', 'delimiter', ';', 'whitespace', ' ');


should pass the assert.

Andy Register <ar12>
Fri 14 Sep 2012 09:44:52 PM UTC, comment #10: 

No worries, I'll try to make something out of it.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 14 Sep 2012 01:06:53 PM UTC, comment #9: 

So maybe that last test isn't quite right after all.

Andy Register <ar12>
Fri 14 Sep 2012 01:04:58 PM UTC, comment #8: 

Sorry, my bad.  I should have written it either \32 or ' '. 

In my past, we used \b (incorrectly) to identify blank spaces in in code documentation.  For the test code I sent, I was simply passing in something other than empty and arbitrarily picked blank.  Then my old habits caught up with me in the comments.

Andy Register <ar12>
Fri 14 Sep 2012 10:59:39 AM UTC, comment #7: 

Anyway, while processing your tests I failed to understand the last (commented) one with "\b" (which if uncommented Just Works).
What exactly did you mean there?

I'll paste in what I made of it (note: double quotes allow \b etc to be properly interpreted ('\b' remains \b, "\b" becomes a backspace char); forward slashes as path separators also work on Windows from within Octave):


%% Test adding whitespace "\b" arg
%!test
tc = [];
tc{1, 1} = "C:/code;";
tc{1, end+1} = " C:/code/meas;";
tc{1, end+1} = "C:/code/sim;";
tc{1, end+1} = "C:/code/utils;";
string = [tc{:}];
c = textscan (string, "%s", "delimiter", ";", "whitespace", "\b");
for k = 1:numel (c{1})
  lh = c{1}{k};
  rh = tc{k};
  rh(rh == ";") = "";
  rh = strtrim (rh);
  assert (strcmp (lh, rh));
end


Philip Nienhuis <philipnienhuis>
Group Member
Thu 13 Sep 2012 08:05:49 PM UTC, comment #6: 

I don't object.

Anonymous
Thu 13 Sep 2012 07:53:29 PM UTC, comment #5: 

All your tests pass with textscan.m & strread.m in the dev source, so this bug has already been fixed earlier on.

Thanks for preparing those tests! always very useful.
I'll prepare a changeset soon to include them; I'd first need to edit style a bit (single -> double quotes etc.)

Do you object to having your email address listed in the changeset?
Like e.g., in the example here:
http://hg.savannah.gnu.org/hgweb/octave/rev/2136343014d5

Philip Nienhuis <philipnienhuis>
Group Member
Thu 13 Sep 2012 05:12:46 PM UTC, comment #4: 

Okay, I attached test_textscan that has some additional test cases ('whitespace', '' included)  When I put the ! in my version of 3.6.2 all the uncommented tests pass.  I left one case commented because it causes an error in 3.6.2 that I didn't feel like fixing since this version is so old and I don't need it.  It should pass using a proper textscan and could be uncommented.

(file #26549)

Andy Register <ar12>
Thu 13 Sep 2012 12:52:58 PM UTC, comment #3: 

I can certainly capture the gozins and gozouts in the calls I am currently making.  I don't know what format is desired but it should be simple enough once I have some guidance.

Andy Register <ar12>
Thu 13 Sep 2012 08:34:20 AM UTC, comment #2: 

Please try the latest textscan.m from mercurial (see comment #1 in bug #37338)
Looking at the code there I think this bug has already been fixed quite a while ago.

My request for whitespace tests still stands; could you use a small part of your data file(s) to create a test case, or send it to me with description of expected results?
And check in strread.m if whitespace tests there cover your needs?

TIA

Philip Nienhuis <philipnienhuis>
Group Member
Wed 12 Sep 2012 11:13:01 PM UTC, comment #1: 

Thanks for reporting, I'll have a look. At first sight I think you are right.

In strread.m (textscan's work horse) similar checks exist.

If you can come up with test cases, please do.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 12 Sep 2012 11:47:19 AM UTC, original submission:  

Line 113 in usr/share/octave/3.6.2/m/io/textscan.m probably needs a ! inserted.

That is,

    if ((isempty (args{ipos+1}) &&  has_str_fmt))

should probably be changed to

    if (! (isempty (args{ipos+1}) &&  has_str_fmt))


This fix worked in the MATLAB code I ported over and it makes the code consistent with the comments.  I also noticed there were no test cases for 'whitespace'.

Thanks

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #26549:  test_textscan.m added by ar12 (2KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ar12 (Posted a comment)
  • -email is unavailable- added by philipnienhuis (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-01-01 philipnienhuis StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2012-09-13 philipnienhuis StatusNeed Info In Progress
    2012-09-13 ar12 Attached File- Added test_textscan.m, #26549
    2012-09-13 philipnienhuis StatusNone Need Info
    2012-09-12 philipnienhuis Assigned toNone philipnienhuis

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code