(string) numbers with commas are not parsed correctly

[techieshark]

(string) numbers with commas are not parsed correctly

[amytych]

Thanks for pointing that out @techieshark!

I think we might be better of with using + for parsing numbers, since indeed it behaves better than parseFloat. I don't think we should go as far as trying to handle formatted numbers, we could mention that in the docs.

What do you think @nickslevine?

[nickslevine]

Thanks for pointing out, @techieshark!

@amytych Seems as if the + operator doesn't fix the problem with the commas in number-strings, unless I'm missing something?

Couldn't hurt to put a warning in the docs to start and then file an enhancement issue for smarter parsing? What do you both think?

[amytych]

Seems as if the + operator doesn't fix the problem with the commas in number

Hehe, yes, I'm aware, I just meant it's better in a way that it returns NaN when it can't parse a string as a number, which seems more appropriate than a totally incorrect number like parseFloat does, e.g:

As for the smarter parsing I'm worried it might be a mine field and out of scope of this library. Dealing with localized formats might be too much of a hustle, as also pointed out in the SO thread linked in the issue it's not trivial to do in a reliable manner. (tldr: commas might mean very different things in formatted strings)

[amytych]

One more note, seems that using + would make it behave similar to pandas.to_numeric() with errors="coerce" setting?

https://repl.it/repls/VainWiltedSeptagon
https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.to_numeric.html

[nickslevine]

@amytych cool cool, that all makes a lot of sense!

[nickslevine]

Just switched parseFloat() with + and added a note re:commas in the README. @techieshark thanks for flagging!

@amytych - I wasn't quite sure how to update the docs website - mind reminding me how the build process works? Thanks!

[amytych]

Great you actioned this @nickslevine, thanks!

I'd still consider a PR route even for such a seemingly small case, some things worth thinking about a bit more here:

• improving tests to cover as many edge cases as possible
• should we consider handling empty strings? +"" === 0 ☹️
• when releasing we should perhaps bump the major version since it might not be backwards compatible?
• worth explaining the behavior a bit more within the parseNums docs as well (or at least repeat what you've put in the main README)?

I'll look into publishing new docs 👍

[nickslevine]

That all makes sense - sound good!