Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distinguishing between different null types #143

Open
kskyten opened this issue Apr 12, 2021 · 3 comments · May be fixed by #166
Open

Distinguishing between different null types #143

kskyten opened this issue Apr 12, 2021 · 3 comments · May be fixed by #166

Comments

@kskyten
Copy link
Contributor

kskyten commented Apr 12, 2021

Currently, these is only one type of null in the code generator. The following code

json = """
[
    {"a": null, "b": 1},
    {"a": null, "b": null}
]
"""

JSON3.generate_exprs(JSON3.generate_type(JSON3.read(json)), mutable=false)

produces this struct

struct Root
    a::Nothing
    b::Union{Nothing, Int64}
end

This is only because the field a does not have enough samples to produce a meaningful type. On the other hand, the values in b are typical in JSON and would be nice to represent with "the software engineer's null" (JuliaLang/julia#22682). I would like the code generator to output something like

const Maybe{T} = Union{Some{T}, Nothing}

struct Root
    a::Missing
    b::Maybe{Int64}
end

or

const Maybe{T} = Union{Some{T}, Nothing}

struct Root
    a::Any
    b::Maybe{Int64}
end
@mcmcgrath13
Copy link
Collaborator

null is currently represented in JSON3 as nothing, which is why the current handling of missingness shows those as the types. There are a couple of places where this is a bit murkier as Nothing is used as a stand-in for missing and perhaps that could be more distinct. However, neither of those use cases would show up in the example you provided as the samples show that null is the only provided option. More samples is the safer answer for that one. Any as the top type for only nothing could more easily be done at the generate_exprs phase - let me think on if there's anything I'm not thinking of there (or maybe that should be opt in/out).

I'm a bit hesitant to introduce too many edge cases/constructs here as these are intended as more of a "starter type" where maybe it's usable out of the box, maybe it needs a bit of editing to be used efficiently. @quinnj what do you think?

Relatedly, Some doesn't seem to currently be supported by StructTypes out of the box:

using JSON3
using StructTypes

struct MyType
    a::Some{Int}
end

StructTypes.StructType(::Type{MyType}) = StructTypes.Struct()

json = """{"a": null}"""

JSON3.read(json, MyType)
# ^ ERROR: ArgumentError: Some{Int64} doesn't have a defined `StructTypes.StructType`

@quinnj
Copy link
Owner

quinnj commented Apr 13, 2021

Hmmm, yeah, I think I agree with @mcmcgrath13 that the type generating machinery is really about getting a good "start" to generating types and not really getting it perfect. The produced files are easily editable.

Yeah, maybe we need to add support for Some somehow; it's a bit late though and my brain is running itself in circles trying to think of what that looks like between StructTypes/JSON3. I'll try to wrap my head around it again in the morning 😴

quinnj added a commit to JuliaData/StructTypes.jl that referenced this issue Apr 14, 2021
Brought up in quinnj/JSON3.jl#143. This is
basically the exact use-case for `CustomStruct`.
@quinnj
Copy link
Owner

quinnj commented Apr 14, 2021

Ok, support for Some added in the StructTypes.jl package: JuliaData/StructTypes.jl#45

quinnj added a commit to JuliaData/StructTypes.jl that referenced this issue Apr 14, 2021
* Define StructType for Some as CustomStruct

Brought up in quinnj/JSON3.jl#143. This is
basically the exact use-case for `CustomStruct`.

* fix tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants