bugGNU Octave - Bugs: bug #34713, str2double() ignores spaces and...

 
 

bug #34713: str2double() ignores spaces and multiple commas

Submitted by:  Philip Nienhuis <philipnienhuis>
Submitted on:  Tue 01 Nov 2011 08:54:36 AM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: ConfirmedAssigned to: None
Originator Name: Philip NienhuisOpen/Closed: Open
Release: devOperating System: Any

Add a New Comment (Rich MarkupRich 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)

Wed 04 Jan 2012 04:02:29 PM UTC, comment #7:

Everyone thinks their bug should have high priority. How is this bug significantly different from other bug reports that it warrents high priority and important severity while so many others are just "normal"?

Does Matlab's str2double function parse commas and periods strings differently depending on locale settings? If not, then I'm confident that we would see bug reports about Matlab compatibility if we chose to do it in Octave.

John W. Eaton <jwe>
Project Administrator
Wed 21 Dec 2011 10:10:16 PM UTC, comment #6:

I agree that this is too early to do. Potentially we could use the relevant code from somewhere else?

In fact, Octave is using GNULib somehow. As far as I understand, its sscanf together with setlocale should have the appropriate functionality. I have no idea however how this then influences other calls. If this is too complicated, I can have a look if its easy to find the relevant code in gnulib.

Kris Thielemans <krthie>
Wed 21 Dec 2011 02:25:55 PM UTC, comment #5:

I think a change to use locales deserves more testing than we can do before the release. A change like you propose could go on the default branch, but I think it needs to be more robust to possible errors. For example, what happens if a locale name is not available? Is an exception thrown? Should we just try known locale names until one succeeds? What if none succeed? Do we throw an error, or just strip commas as we are doing now?

John W. Eaton <jwe>
Project Administrator
Wed 21 Dec 2011 12:20:27 PM UTC, comment #4:

It turns out that we could let istream>> do the comma handling by setting the local appropriatelly by simply removing the lines that remove the comma and setting the local there the something like en_US (see code below).

With this, all current tests succeed and we also have good handling of comma's:

octave> str2double('2,235.6 + 35,564.3i')
ans = 2.2356e+03 + 3.5564e+04i
octave> str2double('2,23,5.6')
ans = NaN

This solution is very elegant, but unfortunately has a major problem: local naming is not set by the standard (on Ubuntu 11.10 I can use "en_US.UTF-8", but Microsoft seems to use "english_US" (see http://msdn.microsoft.com/en-us/library/hzz3tw78.aspx), and it's not clear if en_US is installed on all systems in any case. I have no solution for this.

Of course, arguably str2double should use the system local anyway, but that would seem to break matlab compatibility.

$ hg diff str2double.cc
diff -r e5e74694c690 src/DLD-FUNCTIONS/str2double.cc
--- a/src/DLD-FUNCTIONS/str2double.cc Wed Dec 21 09:08:10 2011 +0000
+++ b/src/DLD-FUNCTIONS/str2double.cc Wed Dec 21 11:34:41 2011 +0000
@@ -227,13 +227,10 @@
{
Complex val (0.0, 0.0);

- std::string str = str_arg;
+ std::istringstream is (str_arg);
+ // handle comma's
+ is.imbue( std::locale("en_US.UTF-8") );

- // FIXME -- removing all commas does too much...
- std::string::iterator se = str.end ();
- se = std::remove (str.begin (), se, ',');
- str.erase (se, str.end ());
- std::istringstream is (str);

Kris Thielemans <krthie>
Wed 21 Dec 2011 08:34:10 AM UTC, comment #3:

Thanks for looking into this.

FYI, ML r2007a does this:

>> str2double ('1,2,3,4')

ans =
1234

>> str2double ('1 2 3 4')

ans =
NaN

>> str2double ('1,234')

ans =
1234

The first looks like a subtle ML bug to me - while commas are to be interpreted as thousands separators, they turn out to concatenate groups of any number of digits in ML r2007a.
(I'd expect only constructs with 3-digit groups to the right of the leftmost comma would be accepted, like this:

>> str2double ('1,200,300,400')

ans =
1.2003e+009
)

Philip Nienhuis <philipnienhuis>
Project Member
Tue 20 Dec 2011 11:13:10 PM UTC, comment #2:

I checked in the following fix for the original problem.

http://hg.savannah.gnu.org/hgweb/octave/rev/b6eeeb67fa3f

Correctly parsing numbers like "1,234" while also disallowing "1,2,3,4" will require more work.

John W. Eaton <jwe>
Project Administrator
Thu 03 Nov 2011 12:06:47 PM UTC, comment #1:

similar weird behaviour alluded to in the bug-title, but no example was given in the original post:

Expected result: NaN

Kris

Kris Thielemans <krthie>
Tue 01 Nov 2011 08:54:36 AM UTC, original submission:

(see thread: https://mailman.cae.wisc.edu/pipermail/octave-maintainers/2011-October/025525.html)

octave:11> str2double ('1 2 3 4')
ans = 1234

where NaN would be expected.

Philip Nienhuis <philipnienhuis>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by krthie (Posted a comment)
  • -unavailable- added by bpabbott (Updated the item)
  • -unavailable- added by philipnienhuis (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 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 04 Jan 2012 04:02:29 PM UTCjweSeverity4 - Important=>3 - Normal
      Priority7 - High=>5 - Normal
    Sat 03 Dec 2011 02:53:24 AM UTCbpabbottPriority5 - Normal=>7 - High
    Mon 28 Nov 2011 09:06:41 PM UTCphilipnienhuisSeverity3 - Normal=>4 - Important
    Thu 03 Nov 2011 01:25:48 AM UTCbpabbottStatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup