quinnj / JSON3.jl

Other
214 stars 47 forks source link

JSON3.pretty() converts Float64 to Int if the float is mathematically equivalent to an integer #227

Open toollu opened 2 years ago

toollu commented 2 years ago

`julia> Foo = Dict("bar"=>0.1 , "baz"=>1.0) Dict{String, Float64} with 2 entries: "bar" => 0.1 "baz" => 1.0

julia> JSON3.pretty(Foo) { "bar": 0.1, "baz": 1 }`

which is odd behaviour and causes (in our use case) downstream problems for teams developing against our API as they expect a certain type.

quinnj commented 2 years ago

Maybe I can ask @jlbosse or @lsaenzt to help take a look at this?

lsaenzt commented 2 years ago

Hi, it looks that the issue is coming from JSON3.read("1.0") that returns 1 as Int. JSON3.pretty writes 1.0 to JSON resulting in "1.0" and then it "JSON3.reads" the result as 1.

a = JSON3.write(1.0)
JSON3.read(a)

@quinnj, JSON3.read function is very complex. If you can point me where to look, I'll give it a try.

lsaenzt commented 2 years ago

@quinnj , I've spent a bit more of time on this. The issue seems to come from these lines in JSON3.read!

elseif (UInt8('0') <= b <= UInt8('9')) || b == UInt8('-') || b == UInt8('+') || (allow_inf && (b == UInt8('N') || b == UInt8('I')))
        float, code, floatpos = Parsers.typeparser(Float64, buf, pos, len, b, Int16(0), Parsers.OPTIONS)
        if code > 0
            !isfinite(float) && !allow_inf && @goto invalid
            @check
            # if, for example, we've already parsed floats in an array, just keep them all as floats and don't check for ints
            if checkint
                fp, ip = modf(float)
                if fp == 0 && Float64(typemin(Int64)) <= float <= Float64(typemax(Int64))
                    # ok great, we know the number is integral and pretty much in Int64 range
                    if -FLOAT_INT_BOUND < float < FLOAT_INT_BOUND
                        # easy case, integer w/ less than or equal to 53-bits of precision
                        int = unsafe_trunc(Int64, float)

The function is truncating to Int64 if the fractional part is 0.

I could not find documentation for Parsers.typeparser.

quinnj commented 2 years ago

Yeah, basically this code parses as a float, then checks some conditions to see if we can truncate to integer, so thing like "number": 10 are parsed as Int instead of Float64. I'm wondering if it would work to have a new keyword argument like truncate_to_int=true that we could set to false from JSON3.pretty to avoid doing the truncation at all.

lsaenzt commented 2 years ago

But that would cause that all "Ints" would be parsed as Floats, right? The issue would persist if the user is expecting the original type.

Maybe Parsers.parsedigits should return the position of the decimal point and if it's zero then we can safely truncate. It sounds as a breaking change for everyone using Parsers.jl, though. I will give it another thought.

quinnj commented 2 years ago

Yeah, you're right. I'm also thinking over alternative solutions; I'll try to experiment over the next few days.

toollu commented 1 year ago

any progress on this? or any suggestions how I might be of help?

nrontsis commented 1 year ago

Just noting that I also came across this issue

chriscoey commented 1 year ago

I too am running into this inconvenient behavior 😄

chriscoey commented 1 year ago

JSON.jl doesn't suffer from this though, so I'll switch to that.

julia> JSON.parse("5.0")
5.0

julia> JSON3.read("5.0")
5
quinnj commented 1 year ago

I've got a "fix" in the works, but it's a big set of changes, so it might take a while to propagate through everywhere.