bugGNU Octave - Bugs: bug #47538, textscan result different from...

 
 

bug #47538: textscan result different from 4.0.0 to dev

Submitter:  Rik <rik5>
Submitted:  Sat 26 Mar 2016 12:00:16 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  lachlan
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 14 Apr 2016 12:24:33 AM UTC, comment #6: 

I applied Lachlan's patch to make ReturnOnError Matlab-compatible with only true/false accepted.  See http://hg.savannah.gnu.org/hgweb/octave/rev/fed1111e1899.

Going back to the motivating example,


textscan ("1 A 3", "%d", 'ReturnOnError', 0)
error: textscan: Read error in field 1 of row 2


This seems alright except shouldn't the row number be 1?  That is probably a numbering issue that deserves its own bug report.

Closing this report.

Rik <rik5>
Group administrator
Sun 10 Apr 2016 09:58:48 AM UTC, comment #5: 

Philip, I agree that the old mode of trying to continue is problematic.  It is probably better just to alert the user that the file is problematic.  If the user needs something more flexible, they should probably do their own parsing.

The attached patch gets rid of the "continue" option entirely, so that we only have the two Matlab-compatible options.

Rik's original example still behaves differently from 4.0.0, but I believe it now does what Matlab does: return only {[1]}.

(file #36888)

Lachlan Andrew <lachlan>
Mon 28 Mar 2016 08:29:32 PM UTC, comment #4: 

@Lachlan
Maybe it is better to forget about old textscan.m's behavior as regards ReturnOnError.

Explanation:
------------
The "old" textscan didn't fully implement ReturnOnError simply because it just couldn't, given the way it worked. So the parameter was accepted but didn't actually work at all in the sense that there as no way to stop textscan.m/strread.m from reading on, as long as the file columns were consistent until EOF.
Hence "not fully implemented" can be perceived as a sort of euphemism :-)

That is different from Matlab's textscan that always quits reading when it encounters a non-matching field, either with or without returning the results til the offending field, depending on ReturnOnError.

The new Octave textscan, like Matlab's textscan, works linearly trough files, so the "old" "intermediate" way isn't applicable anymore.

But if there's a use case for it I wouldn't mind if Octave's textscan could do better than Matlab's. In spite of what I wrote at the top of bug #47553 comment #5 :-)

The risk is of course that textscan encounters an error reading some field for a very good reason (viz. new file section / change of setup of the file, instead of a minor hickup/typo in just one field), and trying to continue reading may only yield error upon error until the end of (a possibly very very long) file.
Maybe "continue" should work as a sort of "job reprieved", i.e., ignore the erroneous field and insert EmptyValue, and allows just one more try, or a limited number of tries (say 5 or 10)? before giving up completely. But this gets very subjective....

Philip Nienhuis <philipnienhuis>
Group Member
Sat 26 Mar 2016 09:46:43 PM UTC, comment #3: 

As I thought, it was a regression from bug #47458.

It is fixed by the attached patch.  However I should probably think a bit more to make sure that this is the "right" fix, and also update some documentation before this is committed.  I hope to post a final patch within a week.

(file #36774)

Lachlan Andrew <lachlan>
Sat 26 Mar 2016 08:38:01 AM UTC, comment #2: 

Yes, this is a bug with deciding when to end -- I keep getting into knots over that.  This may be a side effect of the patch for bug #47458.

Setting "ReturnOnError" to be 0 or 1 (the only matlab-compatible options) gives what I would expect, namely an error or { 1 }.

"continue" is supposed to be essentially what the old textscan.m did, which is why it is the default.  I'll defer to whatever the consensus is.

Now I'll get to debugging :)

Lachlan Andrew <lachlan>
Sat 26 Mar 2016 12:12:20 AM UTC, comment #1: 

Actually the default value of ReturnOnError in Octave is the string "continue", which is documented to mean keep reading even if a conversion specifier doesn't match.

I'm not sure what the expected result is supposed to be with this option, but if you explicitly set ReturnOnError to either true or false I think you get the right answer, hopefully Matlab compatible.

The 4.0 version of textscan says

> warning: textscan: ReturnOnError is not fully implemented


By the way Matlab says that the default value of ReturnOnError is true, and doesn't even mention a value of "continue", so I think this needs changing anyway to be compatible.

https://www.mathworks.com/help/matlab/ref/textscan.html

Mike Miller <mtmiller>
Group Member
Sat 26 Mar 2016 12:00:16 AM UTC, original submission:  

Just an observation, under 4.0.0


textscan ("1 A 3", "%d")
ans =
{
  [1,1] =

    1
    0
    3

}


But with the default branch


textscan ("1 A 3", "%d")
ans =
{
  [1,1] =

    1
    0

}


Maybe something wrong with ReturnOnError?


Rik <rik5>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by rik5 (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-04-14 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2016-04-10 lachlan Attached File- Added bug_47538_textscan_no_continue_on_error.cset, #36888
        StatusIn Progress Patch Submitted
        Assigned toNone lachlan
    2016-03-27 lachlan StatusConfirmed In Progress
    2016-03-26 lachlan Attached File- Added bug_47538_textscan_abort_on_error.cset, #36774
    2016-03-26 mtmiller StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code