Fix String.toFloat

[slava-sh]

• Added handling of NaN and Infinity.
• toFloat "1." and toFloat ".1" now result in an error.

Closes #336.

[Dandandan]

Are you sure NaN / Infinity should even be handled (as they are not available as literals in Elm)?

[Dandandan]

Are you sure NaN / Infinity should even be handled (as they are not available as literals in Elm)?

[slava-sh]

I think that toFloat (toString someFloat) should not throw. There are no NaN and Infinity literals in Haskell and read . show is an identity.

[slava-sh]

I think that toFloat (toString someFloat) should not throw. There are no NaN and Infinity literals in Haskell and read . show is an identity.

[jvoigtlaender]

How is this pull request related to https://github.com/elm-lang/core/pull/343?

[jvoigtlaender]

How is this pull request related to https://github.com/elm-lang/core/pull/343?

[slava-sh]

This PR improves String.toFloat by disallowing dangling dots and adding proper handling of NaN and Infinity:

> String.toFloat ".1"
Err ("could not convert string '.1' to a Float") : Result.Result String Float

> 0 / 0
NaN : Float
> String.toFloat (toString (0 / 0))
Ok NaN : Result.Result String Float

> -1 / 0
-Infinity : Float
> String.toFloat (toString (-1 / 0))
Ok -Infinity : Result.Result String Float

Disallowing dangling dots is reasonable because Elm itself does not support them:

> 1 + .1
-- SYNTAX PROBLEM -------------------------------------------- repl-temp-000.elm

I ran into something unexpected when parsing your code!

3│   1 + .1
          ^

The other PR fixes #336 by making String.toFloat "." return an Err. It does not fix String.toFloat "-.", which is still an Ok NaN.

[slava-sh]

This PR improves String.toFloat by disallowing dangling dots and adding proper handling of NaN and Infinity:

> String.toFloat ".1"
Err ("could not convert string '.1' to a Float") : Result.Result String Float

> 0 / 0
NaN : Float
> String.toFloat (toString (0 / 0))
Ok NaN : Result.Result String Float

> -1 / 0
-Infinity : Float
> String.toFloat (toString (-1 / 0))
Ok -Infinity : Result.Result String Float

Disallowing dangling dots is reasonable because Elm itself does not support them:

> 1 + .1
-- SYNTAX PROBLEM -------------------------------------------- repl-temp-000.elm

I ran into something unexpected when parsing your code!

3│   1 + .1
          ^

The other PR fixes #336 by making String.toFloat "." return an Err. It does not fix String.toFloat "-.", which is still an Ok NaN.

[jvoigtlaender]

Ah, I see, thanks for clarifying. So, just to be sure: Does this PR solve a superset of the problems that the other PR solves? So the "." case is handled here as well?

My original question was because both PRs are mentioned to address #336, and the code changes are overlapping, so there would be a conflict if one gets merged.

Am I right that your position is that the other PR should be closed and this one merged?

What about Evan's comment over there, about first using a regular expression to reject invalid strings? Maybe that would simplify the solution here as well, since after that check has passed there are fewer cases to consider?

[jvoigtlaender]

Ah, I see, thanks for clarifying. So, just to be sure: Does this PR solve a superset of the problems that the other PR solves? So the "." case is handled here as well?

My original question was because both PRs are mentioned to address #336, and the code changes are overlapping, so there would be a conflict if one gets merged.

Am I right that your position is that the other PR should be closed and this one merged?

What about Evan's comment over there, about first using a regular expression to reject invalid strings? Maybe that would simplify the solution here as well, since after that check has passed there are fewer cases to consider?

[slava-sh]

Yes, that's right.

A regex would indeed simplify the code. Also, Evan's regex allows e notation (e.g. toFloat "1.2e3"), which is currently not supported but should probably be.

[slava-sh]

Yes, that's right.

A regex would indeed simplify the code. Also, Evan's regex allows e notation (e.g. toFloat "1.2e3"), which is currently not supported but should probably be.

[mgold]

e notation is valid as a literal and therefore should be valid in String.toFloat. (Similarly, hex literals should be accepted by String.toInt, although they currently are not.)

[mgold]

e notation is valid as a literal and therefore should be valid in String.toFloat. (Similarly, hex literals should be accepted by String.toInt, although they currently are not.)

[jvoigtlaender]

So, @slava-sh, are you willing to revise your pull request by using a regex and also supporting e notation?

[jvoigtlaender]

So, @slava-sh, are you willing to revise your pull request by using a regex and also supporting e notation?

[slava-sh]

Yes, I'll do this.

[slava-sh]

Yes, I'll do this.

[slava-sh]

Done.

[slava-sh]

Done.

[jvoigtlaender]

Cool!

I think this should be part of a bugfix release core-3.0.1, see here, but it's @evancz's to decide.

[jvoigtlaender]

Cool!

I think this should be part of a bugfix release core-3.0.1, see here, but it's @evancz's to decide.

[evancz]

I added this to #721, but it is very unclear that "Fix String.toFloat" is a legitimate title for this suggestion. In any case, it is now tracked in a meta issue so these decisions can be made in a comprehensive way.

[evancz]

I added this to #721, but it is very unclear that "Fix String.toFloat" is a legitimate title for this suggestion. In any case, it is now tracked in a meta issue so these decisions can be made in a comprehensive way.