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 Nov 1 08:54:36 2011  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: None
Originator Name: Philip NienhuisOpen/Closed: Closed
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)

Sat May 16 18:51:29 2015, comment #8:

If I read this bug history correctly, the only remaining issue was whether to treat the comma as a decimal separator depending on locale?

I think we are now pretty sure that we don't want to do that, and took some steps in the most recent version to ensure even more strongly that LC_NUMERIC is set to C.

Matlab apparently ignores all commas in numeric literals, e.g.

and Octave behaves exactly the same way now.

Closing as fixed.

Mike Miller <mtmiller>
Project Administrator
Wed Jan 4 16:02:29 2012, 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 Dec 21 22:10:16 2011, 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 Dec 21 14:25:55 2011, 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 Dec 21 12:20:27 2011, 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 Dec 21 08:34:10 2011, 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 Dec 20 23:13:10 2011, 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 Nov 3 12:06:47 2011, 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 Nov 1 08:54:36 2011, 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 mtmiller (Posted a comment)
  • -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 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat May 16 18:51:29 2015mtmillerStatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
    Wed Jan 4 16:02:29 2012jweSeverity4 - Important=>3 - Normal
      Priority7 - High=>5 - Normal
    Sat Dec 3 02:53:24 2011bpabbottPriority5 - Normal=>7 - High
    Mon Nov 28 21:06:41 2011philipnienhuisSeverity3 - Normal=>4 - Important
    Thu Nov 3 01:25:48 2011bpabbottStatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup