bugGNU Octave - Bugs: bug #37961, textscan : whitespace...

 
 

bug #37961: textscan : whitespace specification doesn't work

Submitter:  None
Submitted:  Wed 19 Dec 2012 06:25:57 PM UTC
   
 
Category:  Interpreter Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  philipnienhuis
Originator Name:  maf88_ Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 3.6.3
Operating System:  * GNU/Linux Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 02 Jan 2013 04:33:33 PM UTC, comment #19: 

Thanks. Those two lines were all this bug report needed:

    http://hg.savannah.gnu.org/hgweb/octave/rev/878623ca15e1

As to the merge issues, they are trivial:

    http://hg.savannah.gnu.org/hgweb/octave/rev/1733bd181cb6

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Sun 30 Dec 2012 09:04:35 PM UTC, comment #18: 

Well, it may not be so very super-easy.

(1) Wouldn't such a fix, on top of the 5 Ben mentioned, interfere with the actual fix from almost a year ago (changeset 14213:a022c04f68cc by Rik) once those 5 are finally merged into the next stable release?

(2) Simply (pre-)defining the variable "whitespace" isn't the way to go as that will induce many obscure problems later on in strread.m
Luckily the actual fix boils down to replacing L. 114
      args{ipos+1} = unique ([" ", whitespace]);
with
      args{ipos+1} = unique ([" ", args{ipos+1}]);

...plus adding a ";" at the end of L.109
(all relative to the Octave-3.6.3 sources)

Philip Nienhuis <philipnienhuis>
Group Member
Sun 30 Dec 2012 04:46:47 PM UTC, comment #17: 

As far as I can see, the actual critical bug on stable is quite easy to fix: just define the whitespace variable before the codepath in which it used undefinedly. It may be easier to just make that small fix.

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Sun 30 Dec 2012 04:34:03 PM UTC, comment #16: 

Jordi, ignoring the style, deprecation, and spell-check changes, there are five changesets (below) that impact the results.  Do you mean to imply that adding all of these is too aggressive?  I tend to think it is, but am worried that a subset might introduce unexpected behavior.


changeset:   14565:98aaebc56d7c
user:        Philip Nienhuis <prnienhuis@@users.sf.net>
date:        Fri Mar 30 18:44:01 2012 +0200
summary:     2012-03-25 Philip Nienhuis <prnienhuis@users.sf.net>

changeset:   14582:e97ec01d4157
user:        Alexander Graf <a.graf@fz-juelich.de>
date:        Fri Apr 27 17:22:07 2012 +0200
summary:     Correct assignment to wrong variable f headerlines processing

changeset:   14799:21197c43f984
user:        Philip Nienhuis <prnienhuis@users.sf.net>
date:        Fri Jun 22 19:09:30 2012 +0200
summary:     Better Matlab compatibility of textscan, bugs #36356 and #36392

changeset:   14800:a922f768ee09
user:        Philip Nienhuis <prnienhuis@users.sf.net>
date:        Fri Jun 22 20:01:35 2012 +0200
summary:     textscan, strread: improved ML compatibility (bug 36398)

changeset:   15263:2136343014d5
user:        Philip Nienhuis <prnienhuis@users.sf.net>
date:        Thu Aug 30 20:52:40 2012 +0200
summary:     bug #37023 (wrong reading of lines starting and/or ending with whitespace)


Ben Abbott <bpabbott>
Group Member
Sun 30 Dec 2012 04:18:48 PM UTC, comment #15: 

Philip, that's a lot of changes:


jordi@Iris:~/coding/vcs/octave-devel/scripts/io$ hg log -r 'branch(default) and "release-3-6-0"::' textscan.m > lol

changeset:   14213:a022c04f68cc
user:        Rik <octave@nomad.inbox5.com>
date:        Wed Jan 18 21:14:36 2012 -0800
summary:     Replace to-be-deprecated strmatch occurrences with alternate code.

changeset:   14214:2fe0f5fa8cc3
user:        Rik <octave@nomad.inbox5.com>
date:        Wed Jan 18 21:15:42 2012 -0800
summary:     Replace to-be-deprecated findstr occurrences with strfind.

changeset:   14363:f3d52523cde1
parent:      14361:8de863b7126b
user:        Rik <octave@nomad.inbox5.com>
date:        Mon Feb 13 07:29:44 2012 -0800
summary:     Use Octave coding conventions in all m-file %!test blocks

changeset:   14552:86854d032a37
user:        John W. Eaton <jwe@octave.org>
date:        Wed Apr 11 22:07:00 2012 -0400
summary:     maint: miscellaneous style fixes for .m files

changeset:   14565:98aaebc56d7c
user:        Philip Nienhuis <prnienhuis@@users.sf.net>
date:        Fri Mar 30 18:44:01 2012 +0200
summary:     2012-03-25 Philip Nienhuis <prnienhuis@users.sf.net>

changeset:   14582:e97ec01d4157
user:        Alexander Graf <a.graf@fz-juelich.de>
date:        Fri Apr 27 17:22:07 2012 +0200
summary:     Correct assignment to wrong variable f headerlines processing

changeset:   14621:1804d5422f61
user:        Rik <octave@nomad.inbox5.com>
date:        Thu May 10 22:18:52 2012 -0700
summary:     doc: Periodic spellcheck of documentation.

changeset:   14799:21197c43f984
user:        Philip Nienhuis <prnienhuis@users.sf.net>
date:        Fri Jun 22 19:09:30 2012 +0200
summary:     Better Matlab compatibility of textscan, bugs #36356 and #36392

changeset:   14800:a922f768ee09
user:        Philip Nienhuis <prnienhuis@users.sf.net>
date:        Fri Jun 22 20:01:35 2012 +0200
summary:     textscan, strread: improved ML compatibility (bug 36398)

changeset:   14853:72b8b39e12be
user:        Rik <octave@nomad.inbox5.com>
date:        Mon Jul 09 10:34:43 2012 -0700
summary:     doc: Periodic grammarcheck of documentation.

changeset:   14868:5d3a684236b0
user:        Rik <octave@nomad.inbox5.com>
date:        Tue Jul 17 07:08:39 2012 -0700
summary:     maint: Use Octave coding conventions for cuddling parentheses in scripts directory

changeset:   15263:2136343014d5
user:        Philip Nienhuis <prnienhuis@users.sf.net>
date:        Thu Aug 30 20:52:40 2012 +0200
summary:     bug #37023 (wrong reading of lines starting and/or ending with whitespace)


We need to be a lot more conservative than this on the stable branch. We can't just copy over all of the changes from default. Do you have any indication which one(s) of these fix the current issue on stable?

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Fri 28 Dec 2012 10:07:22 PM UTC, comment #14: 

Oopsie - I should have written: all csets since Octave 3.6.0 was released

Philip Nienhuis <philipnienhuis>
Group Member
Fri 28 Dec 2012 04:10:24 PM UTC, comment #13: 

Obviously all csets on the files textscan.m / strread.m / textread.m since the 3.6.3 release (there weren't so many).

Tonight & tomorrow I have no time, maybe Sunday I can have a look to sort out what csets are involved.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 28 Dec 2012 02:28:19 AM UTC, comment #12: 

The bug is still open in stable. There should be no codepath that produces a syntax error. Please do not close this bug report until we backport the correct csets back into stable. Any suggestions on what those csets are?

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Thu 27 Dec 2012 11:14:23 PM UTC, comment #11: 

Your analysis is spot-on:

>> ver
------------------------------------------------------------------------------------------------
MATLAB Version: 8.1.0.47 (R2013a)
:
<snip>

>> A = textscan ('XQPCTick;Tag', '%s;%s')
A =
    {1x1 cell}    {0x1 cell}
>> A{:}
ans =
    'XQPCTick;Tag'
ans =
   Empty cell array: 0-by-1


while Octave with 3.7.0+ textscan/strread gives:

octave.exe:1> A = textscan ('XQPCTick;Tag', '%s;%s')
warning: implicit conversion from scalar to sq_string
warning: Ambiguous '☺' specifier next to literal in column
A =
{
  [1,1] =
  {
    [1,1] = XQPCTick;Tag
  }
  [1,2] = {}(0x1)
}


There is something fishy in Octave but I cannot easily see where the first warning comes from. I've fixed the second warning to be more clear (http://hg.savannah.gnu.org/hgweb/octave/rev/4294430ef736).
Anyway the comments (that I added long time ago) in strread.m around L.600 do confirm your analysis: when using a "%s" format you can't expect Octave, or ML, to reliably distinguish the corresponding text field from an immediately following literal.

That, plus different results in r2012b than in r2013a for your second call is adequate to close this bug with "Invalid".

BTW your patch does give some useful hints on how to process repeated whitespace in %s fields. If I get time & need it myself, and jwe doesn't beat me with textcan.cc, I might have another look at it.

Let's leave it to Jordi to decide if he wants a more robust strread/textscan/textread for upcoming 3.6.4.

Philip Nienhuis <philipnienhuis>
Group Member
Thu 27 Dec 2012 09:13:45 PM UTC, comment #10: 

Looking again, I see the OP's syntax is clumsy/ugly.  If the whitespace parameter is dropped then the statement below is in error.


A = textscan ('XQPCTick;Tag', '%s;%s');


The first "%s" will read the entire string.  The will be no semicolon remaining to match the "literal" one and nothing remaining to be read by the second "%s".

The OP's full syntax does work for me with MatlabR2012b


A = textscan ('XQPCTick;Tag', '%s;%s', 'whitespace', ';');
A{:}
ans =     'XQPCTick'
ans =     'Tag'


While I don't see it in Mathworks documentation, it does appear that the format string is parsed while respecting the "whitespace" parameter.  However, for the format string the characters " \t" are also respected.

I agree we shouldn't try to copy this "feature", as it leads to confusing syntax (like the OP's).

Ben Abbott <bpabbott>
Group Member
Thu 27 Dec 2012 04:10:23 PM UTC, comment #9: 

Ben,

There may be a few issues with your changeset, sorry.


(A) I think the first hunk (concerning eol_char) is unwanted. Variable 'eol_char' was only introduced to process the "endofline" parameter for textscan/textread.
strread.m should have nothing to do with explicit EOLs unless:
- called by textread or textscan
- when the user specifies "endofline" (in fact a textscan/textread parameter)
In all other cases "\n" is part of white space as defined in the ML docs (" \b\t\r\n"), and as such is implicitly a delimiter.


(B) (I promise it's the last time I mention this) AFAICS Matlab has no documentation for what happens if there's a literal in a format string that is simultaneously defined as whitespace. I would be hesitant to "fix" such a "bug". See (C) below...

FYI, Test #10 in strread.m is an example of reading white space with %s format conversion specifiers. It boils down to essentially setting whitespace empty and explicitly specifying delimiter(s). That test is based on older ML docs and examples.


(C) In newest ML r2013a (prerelease), the OP's problem (with the text in his original report and his invocation of textscan) gives:

>> fid = fopen ('testfil.txt')

fid =

     3

>> A=textscan(fid,'%s;%s', 'Whitespace', ';')

A =

    {1x1 cell}    {0x1 cell}

>> A{1}

ans =

    'XQPCTick'

>> A{2}

ans =

   Empty cell array: 0-by-1


... so as his examples no longer works in newest ML I even doubt if ML compatibility is a valid issue here. (A fine example of why it is unwise to rely on undocumented ML behavior)

Again, I think the OP simply invoked textscan (1) in a clumsy way with (2) unfortunately an old and less robust version of the textscan/strread/textread trio.


(D) The newest on-line ML docs (r2012b) for strread can even be interpreted as that whitespace MAY only be one of \b \n \r \t \\ \% "'" (intriguingly space itself isn't mentioned, it used to be in older ML docs).


To Jordi:
Yes I don't see any reason not to apply the most recent textscan/textread/strread versions to stable (i.e., for the 3.6.4 release).


BTW we're still waiting for the OP's answers.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 25 Dec 2012 04:34:23 AM UTC, comment #8: 

I've attached a changeset (somewhere between ugly and clumsy).  All tests for strread() pass.  A test needs to be added to capture the problem reported here.

I've also attached a script that tests for the reported problem, when strread() is used instead of textscan().

There remains a problem with textscan.



(file #27149, file #27150)

Ben Abbott <bpabbott>
Group Member
Tue 25 Dec 2012 03:44:51 AM UTC, comment #7: 

Running either Octave's default or stable branch, the original example gives the error below.


warning: strread: unable to parse text or file with given format string


Matlab gives the same result for FMT = "%s;%s" and FMT = "%s %s".  It is the first that gives produces the error for Octave.  Matlab's result is below.


A{1}

ans =

    'XQPCTick'
    '1552111181823'
    '1552327767233'
    '1552570758808'
    '1552819036603'
    '1553037382424'
    '1553238114161'
    '1553456459982'
    '1553687128680'
    '1553905469514'
    '1554100925005'

A{2}

ans =

    'Tag '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '


WIth FMT = "%s %s", neither Octave's stable and default branch throws an error, but each return the same incorrect result.


A =
{
  [1,1] =
  {
    [1,1] = XQPCTick
    [2,1] = 1552111181823
    [3,1] = Throw
    [4,1] = G8
    [5,1] = 1552570758808
    [6,1] = Throw
    [7,1] = G8
    [8,1] = 1553037382424
    [9,1] = Throw
    [10,1] = G8
    [11,1] = 1553456459982
    [12,1] = Throw
    [13,1] = G8
    [14,1] = 1553905469514
    [15,1] = Throw
    [16,1] = G8
  }
  [1,2] =
  {
    [1,1] = Tag
    [2,1] = G8
    [3,1] = 1552327767233
    [4,1] = Throw
    [5,1] = G8
    [6,1] = 1552819036603
    [7,1] = Throw
    [8,1] = G8
    [9,1] = 1553238114161
    [10,1] = Throw
    [11,1] = G8
    [12,1] = 1553687128680
    [13,1] = Throw
    [14,1] = G8
    [15,1] = 1554100925005
    [16,1] = Throw
  }
}


It looks to me as if the problem is with strread(). Looking over strread(), the problem seems to due to two features of the implementation.

  1. trread() ultimately converts all "whitespace" to char(32).  This will lead to problems reading strings that should include spaces.
  2. The current implementation also trims the input string of spaces in a few locations.


The code responsible for parsing the string is complex and difficult for me to follow.  There may be other problems there.



Ben Abbott <bpabbott>
Group Member
Mon 24 Dec 2012 05:59:34 PM UTC, comment #6: 

The point is that there is a bug on stable that is not on default. If the bug is minor enough, fine, we can ignore it, but otherwise, we should backport the fix from default into stable.

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Thu 20 Dec 2012 09:20:03 PM UTC, comment #5: 

Just to get back to the original bug report:
I don't see why a 'whitespace' parameter is used to essentially specify a delimiter.

In both Octave (dev version) and ML r2012a the following works OK on the example:

A = textscan (fid, "%s %s", "Whitespace", "", "delimiter", ";")

Note that

B = textscan (fid, '%s %s %s', 'Whitespace', ';')

won't work on the example as Octave cannot substitute empty values for fields beyond an EOL on incomplete lines (L.1 contains just two fields). A compiled textscan() could...
A way around that is to invoke textscan in two (or more) passes, each time with a different format string, + format repeat count (1st pass) or 'headerlines' param (last pass) or both (intermediate passes, if present).

Ben, as to your observation:
<QUOTE>
whitespace=";" implies that spaces, tabs and such are not whitespace.
</QUOTE>
Correct. The default for whitespace (" \b\t" is replaced by a user-specified value, as it should.
But.... in L.s 124..125 of textscan.m, it also reads:
    ## If there is a string format AND whitespace value = empty,
    ## don't add a space (char(32)) to whitespace
This is based literally on TMW's on-line help for textscan.m. (see whitespace section).

While I do think textscan.m's (or rather strread.m's) whitespace processing can be improved, AFAICS the setup of the call to textscan by the OP was faulty / clumsy in the first place.

I tend to close this bug report with (in decreasing order of preference):
"Invalid" (faulty call to textscan), or
"Works for me" (= with proper call to textscan), or
"Fixed" (as it relates to an old textscan/strread version), or
"Won't fix" (I can't really guarantee that ;-) )
Who takes the pick?

I'll decrease priority in any event.

Off-topic:
I just noted that TMW changed the behavior of textscan: specifying multiple delimiter chars is now supposed to be in a cell array rather than as a character string.

Philip Nienhuis <philipnienhuis>
Group Member
Thu 20 Dec 2012 03:24:35 AM UTC, comment #4: 

Matlab gives;


A{1}

ans =

    'XQPCTick'
    '1552111181823'
    '1552327767233'
    '1552570758808'
    '1552819036603'
    '1553037382424'
    '1553238114161'
    '1553456459982'
    '1553687128680'
    '1553905469514'
    '1554100925005'

A{2}

ans =

    'Tag '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '


I had expected the proper syntax to read the file to be;


B = textscan (fid, '%s %s %s', 'Whitespace', ';');


However, the results from Matlab do not match Octave.  It appears that whitespace=";" implies that spaces, tabs and such are not whitespace.


B{1}

ans =

    'XQPCTick'
    '1552111181823'
    '1552327767233'
    '1552570758808'
    '1552819036603'
    '1553037382424'
    '1553238114161'
    '1553456459982'
    '1553687128680'
    '1553905469514'
    '1554100925005'

B{2}

ans =

    'Tag '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '
    'G8 Throw '

B{3}

ans =

    ''
    ''
    ''
    ''
    ''
    ''
    ''
    ''
    ''
    ''
    ''


Ben Abbott <bpabbott>
Group Member
Wed 19 Dec 2012 08:20:02 PM UTC, comment #3: 

Sure, but OP refers to 3.6.3.

I can't track 'undefined whitespace' around L.114 in the dev version of textscan.m, so that 'whitespace undefined' bug has been fixed already. Perusing the file log I'd say probably > 2 years ago (when was 3.6.0 (last stable version) released?)
Similarly for a missing ';' at L.109.

Anyway I can't reproduce OP's problems in the dev version. I get output but no errors using his files, but obviously I cannot judge if it is good or not. I think not as I get numbers rather than strings. Let the OP give us relevant info.

My point about ';'-literal and defining it as whitespace still stands.

To the OP (maf88_): advice: grab textscan.m, textread.m and strread.m (they are closely related) from here:
  http://hg.savannah.gnu.org/hgweb/octave/file/1e9a6285acc4/scripts/io
and swap them for the versions you have in /usr/local/share/octave/3.6.3/m/io/
,try again and report back.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 19 Dec 2012 07:18:45 PM UTC, comment #2: 

The bug is genuine, there is a whitespace variable in this code path that isn't defined here. Looks like a cut-and-paste error.

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Wed 19 Dec 2012 06:59:33 PM UTC, comment #1: 

Try with small caps ('whitespace') and report back, pls.
textscan's / strread's / textread's parameters are case-sensitive.

BTW having a "literal' ';' between the format conversion specifiers and at the same time declaring it as whitespace may lead to unpredictable results. What does Matlab return?

Also note that your first line will come out differently from the rest. Use 'headerlines' parameter? delimiters will implicitly be ' \n' (space & newline)

The fmt echo is stray, I'll look into this.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 19 Dec 2012 06:25:57 PM UTC, original submission:  

Hello,

The file i want to parse :
XQPCTick;Tag
1552111181823;G8  Throw
1552327767233;G8  Throw
1552570758808;G8  Throw
1552819036603;G8  Throw
1553037382424;G8  Throw
1553238114161;G8  Throw
1553456459982;G8  Throw
1553687128680;G8  Throw
1553905469514;G8  Throw
1554100925005;G8  Throw

The function I use
 
A=textscan(fid,'%s;%s', 'Whitespace', ';');

And the error I get:

 
The error i get :
 
fmt =
{
  [1,1] = %s;
  [1,2] = %s
}
`whitespace' undefined near line 114 column 37
error: called from `textscan' in file /usr/local/share/octave/3.6.3/m/io/textscan.m near line 114, column 21
error: called from:
error:   /usr/local/share/octave/3.6.3/m/miscellaneous/run.m at line 55, column 7

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #27149:  changeset.patch added by bpabbott (2KiB - application/octet-stream)
file #27150:  go_37961.m added by bpabbott (1KiB - application/octet-stream)
file #27127:  load_tagstream.m added by None (1KiB - text/x-objcsrc - The code (not mine though).)
file #27128:  P3_2_8_p29.csv added by None (987KiB - text/csv - The code (not mine though).)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bpabbott (Posted a comment)
  • -email is unavailable- added by jordigh (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 15 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-01-02 jordigh StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2012-12-28 jordigh StatusInvalid / Not an Octave Bug Confirmed
        Open/ClosedClosed Open
    2012-12-27 philipnienhuis StatusPatch Submitted Invalid / Not an Octave Bug
        Open/ClosedOpen Closed
    2012-12-25 bpabbott Attached File- Added changeset.patch, #27149
        Attached File- Added go_37961.m, #27150
        StatusConfirmed Patch Submitted
    2012-12-20 philipnienhuis Severity3 - Normal 2 - Minor
    2012-12-19 jordigh StatusNeed Info Confirmed
    2012-12-19 philipnienhuis StatusNone Need Info
        Assigned toNone philipnienhuis
    2012-12-19 None Attached File- Added load_tagstream.m, #27127
        Attached File- Added P3_2_8_p29.csv, #27128

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code