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

 
 

bug #47538: textscan result different from 4.0.0 to dev

Submitted by:  Rik <rik5>
Submitted on:  Sat 26 Mar 2016 12:00:16 AM UTC  
 
Category: Octave FunctionSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Regression
Status: FixedAssigned to: Lachlan Andrew <lachlan>
Originator Name: Open/Closed: Closed
Release: devOperating System: GNU/Linux

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission 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,

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>
Project 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>
Project MemberIn charge of this item.
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>
Project 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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project Administrator
Sat 26 Mar 2016 12:00:16 AM UTC, original submission:

Just an observation, under 4.0.0

But with the default branch

Maybe something wrong with ReturnOnError?

Rik <rik5>
Project Administrator

 

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

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by lachlan (Posted a comment)
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by rik5 (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 14 Apr 2016 12:24:51 AM UTCrik5StatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Sun 10 Apr 2016 09:58:48 AM UTClachlanAttached File-=>Added bug_47538_textscan_no_continue_on_error.cset, #36888
      StatusIn Progress=>Patch Submitted
      Assigned toNone=>lachlan
    Sun 27 Mar 2016 04:08:27 AM UTClachlanStatusConfirmed=>In Progress
    Sat 26 Mar 2016 09:46:43 PM UTClachlanAttached File-=>Added bug_47538_textscan_abort_on_error.cset, #36774
    Sat 26 Mar 2016 12:12:20 AM UTCmtmillerStatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup1