Fixed #289: Float64 trucating #292
Conversation
Codecov Report
@@ Coverage Diff @@
## master #292 +/- ##
==========================================
+ Coverage 92.17% 92.18% +<.01%
==========================================
Files 9 9
Lines 1892 1894 +2
==========================================
+ Hits 1744 1746 +2
Misses 103 103
Partials 45 45
Continue to review full report at Codecov.
|
marshal.go
Outdated
| @@ -422,7 +422,7 @@ func (e *Encoder) valueToToml(mtype reflect.Type, mval reflect.Value) (interface | |||
| case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: | |||
| return mval.Uint(), nil | |||
| case reflect.Float32, reflect.Float64: | |||
| return mval.Float(), nil | |||
| return strconv.ParseFloat(fmt.Sprintf("%v", mval.Interface()), 64) | |||
There was a problem hiding this comment.
This change was necessary because mval.Float() convert a reflect.Value of float32 to float64 with more decimal places of original data. I don't know if this is the best solution, I'm wating for reviews
Example:
[12.3,45.6,78.9]
was converted to
[12.300000190734863,45.599998474121094,78.9000015258789]
There was a problem hiding this comment.
Even though I think this is still technically correct, it is probably counter intuitive.
Your change looks good, but I would replace it to only apply the logic on the float32 case:
case reflect.Float32:
return strconv.ParseFloat(strconv.FormatFloat(float64(mval.Interface().(float32)), 'f', -1, 32), 64)
case reflect.Float64:
return mval.Float(), nilMaybe there is a better way, but this is the limit of my knowledge of floats handling in go :(
|
Merged #293, which addresses the same problem, but I believe with a more correct approach. Thank you for your help, sorry for the duplicate work! |
Fixed #289
This PR change the bitSize of FormatFloat to string of 32 to 64. Maybe should be useful add a case statement to FormatFloat of float32 to string, but I don't implement because I was in doubt.