adds test and workaround for 0.5 issue with printing Float32s #172
Conversation
| @@ -251,6 +251,9 @@ end | |||
| @test sprint(JSON.print, [NaN]) == "[null]" | |||
| @test sprint(JSON.print, [Inf]) == "[null]" | |||
|
|
|||
| # check for issue #163 | |||
| @test isapprox(JSON.parse(json(Float32(2.1e-8))), 2.1e-8) | |||
There was a problem hiding this comment.
Why are we testing isapprox here? Does converting to Float64 first not roundtrip properly?
There was a problem hiding this comment.
That's correct. In fact 2.1f8 != 2.1e8. I could probably play with numbers to find one that's more amenable to lossless round-tripping, but that doesn't feel any cleaner than using isapprox.
There was a problem hiding this comment.
Could you do Float32(JSON.parse(json(2.1f-8))) == 2.1f-8?
There was a problem hiding this comment.
ah, good idea. that seems to work on my local tests. Let's make sure CI agrees...
|
LGTM. Do you need this tagged soon? |
|
Soon would be good. If this has bitten me and @kshramt it's probably bitten other people as well, and the error that shows up in Plotly is really cryptic so people won't know where to look for what's wrong. |
| @@ -149,6 +149,12 @@ function _writejson(io::IO, state::State, s::AbstractString) | |||
| Base.print(io, '"') | |||
| end | |||
|
|
|||
| # workaround for issue in Julia 0.5.x where Float32 values are printed as | |||
| # 3.4f-5 instead of 3.4e-5 | |||
| if v"0.5-" <= VERSION < v"0.6.0-dev.788" | |||
There was a problem hiding this comment.
I don't think it matters. This change doesn't break anything, even on v0.4, so the lower bound isn't necessary.
There was a problem hiding this comment.
But I believe the relevant commit is JuliaLang/julia@fc75f4d, which is 0.5.0-rc0+138.
|
Woot. thanks for the quick turnaround! |
|
check http://pkg.julialang.org/pulse.html in the morning in case any dependency statuses change, just made it into tonight's run |
|
@tkelman Checked. Looks fine. |
Fixes #163
I'm not super familiar with how the JSON serialization is expected to work, particularly how strict it's supposed to be with round-tripping floating-point numbers. LMK if just converting to a Float64 for JSON serialization is a bad idea.
Related:
JuliaLang/julia#17720
JuliaLang/julia#18053
JuliaLang/julia#18682
JuliaPlots/PlotlyJS.jl#78