bhftbootcamp / Serde.jl

Serde is a Julia library for (de)serializing data to/from various formats. The library offers a simple and concise API for defining custom (de)serialization behavior for user-defined types
Apache License 2.0
48 stars 9 forks source link

I47 fix CSV deserialization of Union{Nulltype,AnyType} #52

Closed RongkunWang closed 4 months ago

RongkunWang commented 4 months ago

First attempt to contribute to a Julia package. Fix #47.

Used the code in the issue to create a test.

Pull request checklist

gryumov commented 4 months ago

@RongkunWang Hi, thanks for the PR! Looks good, but can you handle this case too?

using Serde

j = """
    {
        "z":1,
        "x": null
    }
"""

struct DataN
    z::Int
    x::Union{String,Nothing}
end

struct DataM
    z::Int
    x::Union{String,Missing}
end

julia> Serde.deser(DataN, Serde.parse_json(j, null = nothing))
DataN(1, nothing)

julia> Serde.deser(DataN, Serde.parse_json(j, null = missing))
ERROR: WrongType: for 'DataN' value 'missing' has wrong type 'x::Missing', must be 'x::Union{Nothing, String}'

julia> Serde.deser(DataM, Serde.parse_json(j, null = nothing))
DataM(1, missing)

julia> Serde.deser(DataM, Serde.parse_json(j, null = missing))
DataM(1, missing)
RongkunWang commented 4 months ago

Hi @gryumov ,

I see, the nulltype() will always be the correct type from the union so I should have just used that with your suggestion.

So now I'm applying the same technique for the other dispatch of deser.

codecov-commenter commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71.72%. Comparing base (7f36b84) to head (125c89a).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #52 +/- ## ========================================== - Coverage 71.78% 71.72% -0.06% ========================================== Files 24 24 Lines 1056 1054 -2 ========================================== - Hits 758 756 -2 Misses 298 298 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

gryumov commented 4 months ago

@RongkunWang, thanks for the bug fix