Repository: Roger-luo/Configurations.jl (julia)

Issue:
It seems the nested field of value `nothing` is not supported
MWE:

```
julia> @option struct OptionA
                         name::Union{Nothing,String}=nothing
                         int::Int = 1
                     end

julia> @option struct OptionB
                         name::Union{Nothing,OptionA}=nothing
                         int::Int = 1
                     end

julia> from_dict(OptionB, Dict{String,Any}("name"=>nothing))
ERROR: option name must be specified using an AbstractDict
Stacktrace:
  [1] error(s::String)
    @ Base ./error.jl:33
  [2] from_kwargs_option_key!(f::Configurations.var"#13#15"{Symbol, Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}}, d::Dict{String, Any}, #unused#::Type{OptionA}, name::Symbol, key::Symbol, kw::Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ Configurations ~/.julia/packages/Configurations/CguPm/src/parse.jl:281
  [3] from_kwargs_option_key!(f::Configurations.var"#13#15"{Symbol, Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}}, d::Dict{String, Any}, #unused#::Type{Union{Nothing, OptionA}}, name::Symbol, key::Symbol, kw::Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ Configurations ~/.julia/packages/Configurations/CguPm/src/parse.jl:291
  [4] #12
    @ ~/.julia/packages/Configurations/CguPm/src/parse.jl:250 [inlined]
  [5] foreach_keywords!(f::Configurations.var"#12#14"{Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}, Dict{String, Any}, Nothing}, list::Dict{String, Any}, #unused#::Type{OptionB})
    @ Configurations ~/.julia/packages/Configurations/CguPm/src/parse.jl:355
  [6] #unsafe_from_underscore_kwargs!#11
    @ ~/.julia/packages/Configurations/CguPm/src/parse.jl:247 [inlined]
  [7] unsafe_from_underscore_kwargs!
    @ ~/.julia/packages/Configurations/CguPm/src/parse.jl:247 [inlined]
  [8] #from_underscore_kwargs!#10
    @ ~/.julia/packages/Configurations/CguPm/src/parse.jl:243 [inlined]
  [9] from_underscore_kwargs! (repeats 2 times)
    @ ~/.julia/packages/Configurations/CguPm/src/parse.jl:242 [inlined]
 [10] from_dict(::Type{OptionB}, d::Dict{String, Any}; kw::Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ Configurations ~/.julia/packages/Configurations/CguPm/src/parse.jl:29
 [11] from_dict(::Type{OptionB}, d::Dict{String, Any})
    @ Configurations ~/.julia/packages/Configurations/CguPm/src/parse.jl:29
 [12] top-level scope
    @ REPL[27]:1
```

Pull request description:
fix #50 

- I realize for serialization we should include defaults by default, thus change the default to `true`, this is breaking
- we shouldn't error while merging kwargs in the dict

Patch:
diff --git a/docs/src/quick-start.md b/docs/src/quick-start.md
index 4fea599..bf3047b 100644
--- a/docs/src/quick-start.md
+++ b/docs/src/quick-start.md
@@ -224,6 +224,61 @@ For other formats, as long as you can convert them to a subtype of `AbstractDict
 you can always convert it to the option type you just defined via `from_dict`, however
 for the sake of simplicity Configurations will not ship such functionality with it.
 
+## Write to TOML
+To write the option struct to a TOML file, simply use the `to_toml` function
+
+```julia
+julia> to_toml(option; include_defaults=false) # write to a String
+
+julia> to_toml("test.toml", option; include_defaults=false) # write to a file
+```
+
+You may also be interested in the docstring of `to_toml`
+
+```@docs
+to_toml
+```
+
+## Write to YAML
+
+To write the option struct to other formats, you need to convert it to a dictionary type
+first via `to_dict`
+
+```@docs
+to_dict
+```
+
+Then you can use YAML package to write the dict to a YAML file
+
+```julia
+julia> using YAML, Configurations
+
+julia> d = to_dict(your_option, YAMLStyle)
+
+julia> YAML.write_file("myfile.yaml", d)
+```
+
+## Write to JSON
+
+or for JSON, we recommend using [JSON](https://github.com/JuliaIO/JSON.jl) or [JSON3](https://github.com/quinnj/JSON3.jl) to write the file as following
+
+for `JSON`
+
+```julia
+julia> using JSON, Configurations
+
+julia> d = to_dict(your_option, JSONStyle)
+
+julia> open("file.json", "w") do f
+           JSON.print(f, d)
+       end
+```
+
+## Write to other formats
+
+For other formats, you can convert your option struct to an `OrderedDict{String, Any}` via
+[`to_dict`](@ref) then serialize the dictionary to your desired format.
+
 ## Type Conversion
 
 Since markup languages usually do not support arbitrary Julia types, thus, one may find the `from_dict`
diff --git a/src/Configurations.jl b/src/Configurations.jl
index 6d2a853..2cec527 100644
--- a/src/Configurations.jl
+++ b/src/Configurations.jl
@@ -23,7 +23,10 @@ export no_default,
     to_dict,
     DuplicatedFieldError,
     DuplicatedAliasError,
-    InvalidKeyError
+    InvalidKeyError,
+    TOMLStyle,
+    YAMLStyle,
+    JSONStyle
 
 include("types.jl")
 include("convert.jl")
diff --git a/src/parse.jl b/src/parse.jl
index e1d0204..15fc3fd 100644
--- a/src/parse.jl
+++ b/src/parse.jl
@@ -189,7 +189,7 @@ function from_dict_inner(::Type{T}, @nospecialize(d)) where T
         key = string(each)
         type = fieldtype(T, each)
         default = field_default(T, each)
- 
+        
         if default === no_default
             if type isa Union
                 pick = pick_union(type, d[key])
@@ -277,9 +277,8 @@ function from_kwargs_option_key!(f, d::AbstractDict, ::Type{T}, name::Symbol, ke
     
     if is_option(T)
         field_d = OrderedDict{String, Any}()
-        if haskey(d, name_str)
-            d[name_str] isa AbstractDict || error("option $key_str must be specified using an AbstractDict")
-            field_d = merge!(field_d, d[name_str])
+        if haskey(d, name_str) && (d_value = d[name_str]) isa AbstractDict
+            field_d = merge!(field_d, d_value)
         end
 
         f(field_d, T) # recurse into subfields
diff --git a/src/serialize.jl b/src/serialize.jl
index 7bb6b81..32a0576 100644
--- a/src/serialize.jl
+++ b/src/serialize.jl
@@ -2,31 +2,76 @@ function assert_option(x)
     is_option(x) || error("$(typeof(x)) is not an option type")
 end
 
-@option struct ConvertOption
-    include_defaults::Bool=false
+@option struct ToDictOption
+    include_defaults::Bool=true
     exclude_nothing::Bool=false
 end
 
 """
-    to_dict(option; include_defaults=false, exclude_nothing=false) -> OrderedDict
+    TOMLStyle::ToDictOption
 
-Convert an object to an `OrderedDict`. 
+Predefined option for TOML compatible [`to_dict`](@ref) option.
+"""
+const TOMLStyle = ToDictOption(include_defaults=true, exclude_nothing=true)
+
+"""
+    YAMLStyle::ToDictOption
+
+Predefined option for YAML compatible [`to_dict`](@ref) option.
+"""
+const YAMLStyle = ToDictOption(include_defaults=true, exclude_nothing=false)
+
+"""
+    JSONStyle::ToDictOption
+
+Predefined option for JSON compatible [`to_dict`](@ref) option.
+"""
+const JSONStyle = ToDictOption(include_defaults=true, exclude_nothing=false)
+
+"""
+    to_dict(x; include_defaults=true, exclude_nothing=false) -> OrderedDict
+
+Convert an object `x` to an `OrderedDict`.
 
 # Kwargs
 
-- `include_defaults`: include the default value.
+- `include_defaults`: include the default value, default is `true`.
 - `exclude_nothing`: exclude fields that have value `nothing`,
     this supersedes `include_defaults` when they are both `true`.
 
+# Format Compatibilty
+
+When mapping an option struct from Julia to TOML/YAML/JSON/etc. format,
+there are some subtle semantic compatibilty one need to deal with, we provide
+some convenient predefined conversion option constants as [`TOMLStyle`](@ref),
+[`YAMLStyle`](@ref), [`JSONStyle`](@ref).
+
 !!! tips
     `to_dict` does not export fields that are of the same values as the defaults. 
     In most cases, this should be the default behaviour, and users should not use
     `include_defaults`, however,  this can be overridden by changing `include_defaults`
     to `true`.
 """
-function to_dict(x; include_defaults::Bool=false, exclude_nothing::Bool=false)
+function to_dict(x; kw...)
+    to_dict(x, ToDictOption(;kw...))
+end
+
+"""
+    to_dict(x, option::ToDictOption) -> OrderedDict
+
+Convert an object `x` to an `OrderedDict` with `ToDictOption` specified.
+
+# Example
+
+```julia
+to_dict(x, TOMLStyle) # TOML compatible
+to_dict(x, YAMLStyle) # YAML compatible
+to_dict(x, JSONStyle) # JSON compatible
+```
+"""
+function to_dict(x, option::ToDictOption)
     assert_option(x)
-    return to_dict(typeof(x), x, ConvertOption(include_defaults, exclude_nothing))
+    return to_dict(typeof(x), x, option)
 end
 
 """
@@ -62,7 +107,7 @@ a `String` for all kinds of option types.
 Configurations.to_dict(::Type, x::VersionNumber) = string(x)
 ```
 """
-function to_dict(::Type{T}, x, option::ConvertOption) where T
+function to_dict(::Type{T}, x, option::ToDictOption) where T
     if is_option(x)
         return _option_to_dict(x, option)
     else
@@ -73,7 +118,7 @@ end
 to_dict(::Type, x) = x
 
 # handle list of options as builtin
-function to_dict(::Type{T}, x::Vector, option::ConvertOption) where T
+function to_dict(::Type{T}, x::Vector, option::ToDictOption) where T
     if is_option(eltype(x))
         return map(p->to_dict(T, p, option), x)
     else
@@ -81,7 +126,7 @@ function to_dict(::Type{T}, x::Vector, option::ConvertOption) where T
     end
 end
 
-function _option_to_dict(x, option::ConvertOption)
+function _option_to_dict(x, option::ToDictOption)
     assert_option(x)
 
     d = OrderedDict{String, Any}()
@@ -135,10 +180,31 @@ end
 Convert an instance `option` of option type to TOML and write it to `IO`. See [`to_dict`](@ref)
 for other valid keyword options. See also `TOML.print` in the stdlib for the explaination of
 `sorted`, `by` and `f`.
+
+# Exclude `nothing`
+
+In TOML specification, there is [no null type](https://github.com/toml-lang/toml/issues/802). One
+should exclude the field if it is not specified (of value `nothing` in Julia). In `to_toml` the option
+`exclude_nothing` is always `true`.
+
+In most cases, `nothing` is used with another type to denote optional or not specified field,
+thus one should always put a default
+value `nothing` to the option struct, e.g
+
+One should define
+
+```julia
+@option struct OptionX
+    a::Union{Nothing, Int} = nothing
+    b::Maybe{Int} = nothing
+end
+```
+
+Here `Maybe{T}` is a convenient alias of `Union{Nothing, T}`.
 """
 function to_toml(f, io::IO, x; sorted::Bool=false, by=identity, kw...)
     is_option(x) || error("argument is not an option type")
-    d = to_dict(x; kw...)
+    d = to_dict(x; exclude_nothing=true, kw...)
     return TOML.print(f, io, d; sorted=sorted, by=by)
 end
 


Test patch:
diff --git a/test/runtests.jl b/test/runtests.jl
index c5f5b2b..790cba1 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -5,6 +5,7 @@ using Configurations: to_dict, from_kwargs, from_dict, alias,
     PartialDefault, field_keywords
 using OrderedCollections
 using Test
+using TOML
 
 "Option A"
 @option "option_a" struct OptionA
@@ -94,18 +95,18 @@ end
 @testset "to_dict" begin
     @test_throws ErrorException to_dict("aaa")
     @test to_dict(option) == dict1
-    @test to_dict(from_dict(OptionB, dict2)) == dict2
+    @test to_dict(from_dict(OptionB, dict2); include_defaults=false) == dict2
     @test to_dict(from_dict(OptionB, dict3); include_defaults=true) == dict3
-    @test to_dict(from_dict(OptionB, dict3)) == dict4
+    @test to_dict(from_dict(OptionB, dict3); include_defaults=false) == dict4
     @test to_dict(OptionF(); include_defaults=true, exclude_nothing=true) == OrderedDict{String, Any}("int"=>1)
     @test to_dict(OptionF(); include_defaults=true, exclude_nothing=false) == OrderedDict{String, Any}("name"=>nothing, "int"=>1)
 end
 
 @testset "to_toml" begin
-    @test to_toml(option) == "float = 0.33\n\n[opt]\nname = \"Roger\"\nint = 2\n"
-    to_toml("test.toml", option)
+    @test to_toml(option; include_defaults=false) == "float = 0.33\n\n[opt]\nname = \"Roger\"\nint = 2\n"
+    to_toml("test.toml", option; include_defaults=false)
     @test read("test.toml", String) == "float = 0.33\n\n[opt]\nname = \"Roger\"\nint = 2\n"
-    @test to_toml(option3) == "[opt]\nname = \"Roger\"\nint = 2\n"
+    @test to_toml(option3; include_defaults=false) == "[opt]\nname = \"Roger\"\nint = 2\n"
     @test to_toml(option3; include_defaults=true) == "float = 0.3\n\n[opt]\nname = \"Roger\"\nint = 2\n"
 end
 
@@ -130,10 +131,6 @@ end
     @test d["name"] == "AAA"
     @test d["opt"]["name"] == "Roger"
 
-    # error for wrong specification when we can't overwrite
-    d = OrderedDict{String, Any}("opt"=>"AAA")
-    @test_throws ErrorException Configurations.from_kwargs!(d, OptionG; name="AAA")
-
     @testset "from_field_kwargs" begin
         # error for ambiguious keyword
         @test_throws DuplicatedFieldError Configurations.from_field_kwargs(OptionG;name="AAA")
@@ -231,7 +228,7 @@ end
     @test option.options[2] == OptionA("b", 2)
     @test option.options[3] == OptionA("c", 3)
 
-    @test to_dict(option) == OrderedDict{String, Any}(
+    @test to_dict(option; include_defaults=false) == OrderedDict{String, Any}(
         "options" => OrderedDict{String, Any}[
             OrderedDict("name" => "a"),
             OrderedDict("name" => "b", "int" => 2),
@@ -239,7 +236,7 @@ end
         ]
     )
 
-    to_dict(VectorOfNumbers([1, 2, 3])) == OrderedDict{String, Any}(
+    to_dict(VectorOfNumbers([1, 2, 3]); include_defaults=false) == OrderedDict{String, Any}(
         "list" => [1, 2, 3]
     )
 end
@@ -329,7 +326,7 @@ end
 end
 
 @testset "to_dict nothing conversion" begin
-    d = to_dict(Julia())
+    d = to_dict(Julia(); include_defaults=false)
     @test !haskey(d, "versions")
     @test !haskey(d, "active")
 end
@@ -491,7 +488,7 @@ end
 end
 
 @testset "Dict convertion for Union{Nothing, OptionType}" begin
-    @test to_dict(UnionNothing()) == OrderedDict{String, Any}()
+    @test to_dict(UnionNothing(); include_defaults=false) == OrderedDict{String, Any}()
     @test to_dict(UnionNothing(UnionConvertA(1))) == OrderedDict{String, Any}(
         "info" => OrderedDict{String, Any}(
             "a" => 1,
@@ -620,3 +617,30 @@ end
     println(DuplicatedFieldError(:name, OptionA))
     println(DuplicatedAliasError("alias"))
 end
+
+module Issue50
+
+using Configurations
+
+@option struct OptionA
+    name::Maybe{String}=nothing
+    int::Int = 1
+end
+
+@option struct OptionB
+    name::Maybe{OptionA}=nothing
+    int::Int = 1
+end
+
+end
+
+@testset "to_dict style" begin
+    option = Issue50.OptionB()
+    @test to_dict(option, TOMLStyle) == to_dict(option; include_defaults=true, exclude_nothing=true)
+    @test to_dict(option, YAMLStyle) == to_dict(option; include_defaults=true, exclude_nothing=false)
+    @test to_dict(option, JSONStyle) == to_dict(option; include_defaults=true, exclude_nothing=false)
+end
+
+@testset "#50" begin
+    @test from_dict(Issue50.OptionB, Dict{String,Any}("name"=>nothing)) == Issue50.OptionB()
+end