search

JuliaIO / JLD2.jl

HDF5-compatible file format in pure Julia
Other
547 stars 85 forks source link

struct upgrade pathway #439

Closed JonasIsensee closed 1 year ago

JonasIsensee commented 1 year ago

This PR implements an experimental way of upgrading stored structs to an updated version. Example:

julia> struct A
       x::Int
       y::Float64
       end

julia> save_object("test.jld2", A(1,2.0))

julia> load_object("test.jld2")
A(1, 2.0)

julia> struct A2
           x::Float64
           y::Float64
           z::Float64 # x*y
       end

julia> JLD2.rconvert(::Type{A2}, nt::NamedTuple) = A2(Float64(nt.x), nt.y, nt.x*nt.y)

julia> JLD2.load("test.jld2", "single_stored_object"; typemap=Dict("Main.A" => JLD2.Upgrade(A2)))
A2(1.0, 2.0, 2.0)

API is clumsy but it was rather easy to implement and could potentially be powerful for some applications.

Please test and give feedback if you're interested.

codecov[bot] commented 1 year ago

Codecov Report

Base: 84.07% // Head: 84.01% // Decreases project coverage by -0.05% :warning:

Coverage data is based on head (2a4cc3d) compared to base (3a6a763). Patch coverage: 83.33% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #439 +/- ## ========================================== - Coverage 84.07% 84.01% -0.06% ========================================== Files 30 30 Lines 4119 4135 +16 ========================================== + Hits 3463 3474 +11 - Misses 656 661 +5 ``` | [Impacted Files](https://codecov.io/gh/JuliaIO/JLD2.jl/pull/439?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaIO) | Coverage Δ | | |---|---|---| | [src/datasets.jl](https://codecov.io/gh/JuliaIO/JLD2.jl/pull/439/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaIO#diff-c3JjL2RhdGFzZXRzLmps) | `90.90% <60.00%> (-0.17%)` | :arrow_down: | | [src/data/reconstructing\_datatypes.jl](https://codecov.io/gh/JuliaIO/JLD2.jl/pull/439/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaIO#diff-c3JjL2RhdGEvcmVjb25zdHJ1Y3RpbmdfZGF0YXR5cGVzLmps) | `76.67% <91.66%> (+0.24%)` | :arrow_up: | | [src/JLD2.jl](https://codecov.io/gh/JuliaIO/JLD2.jl/pull/439/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaIO#diff-c3JjL0pMRDIuamw=) | `89.70% <100.00%> (+0.05%)` | :arrow_up: | | [src/bufferedio.jl](https://codecov.io/gh/JuliaIO/JLD2.jl/pull/439/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaIO#diff-c3JjL2J1ZmZlcmVkaW8uamw=) | `85.22% <0.00%> (-1.14%)` | :arrow_down: | | [src/mmapio.jl](https://codecov.io/gh/JuliaIO/JLD2.jl/pull/439/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaIO#diff-c3JjL21tYXBpby5qbA==) | `90.54% <0.00%> (-0.74%)` | :arrow_down: | | [src/loadsave.jl](https://codecov.io/gh/JuliaIO/JLD2.jl/pull/439/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaIO#diff-c3JjL2xvYWRzYXZlLmps) | `95.65% <0.00%> (+0.04%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaIO). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaIO)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

dpinol commented 1 year ago

@JonasIsensee thanks, this is exactly want I need Sorry for the late reply, I was busy with a release.

I found a corner case required by my project:

using JLD2

Base.@kwdef mutable struct A
    x::Int
    y::Float64
end

Base.@kwdef mutable struct B
    a::Vector{A}
end

function JLD2.rconvert(::Type{A}, nt::NamedTuple)
    @info "loading from NamedTuple"
    A(; nt...)
end

save_object("test.jld2", B([A(; x=1, y=2.0)]))
load_object("test.jld2")

JLD2.load("test.jld2", "single_stored_object"; typemap=Dict("Main.A" => JLD2.Upgrade(A)))

The last line causes the following error. It does not happen if B just contains a field of type A. best regards

Error encountered while load FileIO.File{FileIO.DataFormat{:JLD2}, String}("test.jld2").

Fatal error:
ERROR: TypeError: in Type, in parameter, expected Type, got a value of type JLD2.Upgrade
Stacktrace:
  [1] read_data(f::JLD2.JLDFile{JLD2.MmapIO}, rr::JLD2.ReadRepresentation{Any, JLD2.RelOffset}, read_dataspace::Tuple{JLD2.ReadDataspace, JLD2.RelOffset, JLD2.DataLayout, JLD2.FilterPipeline}, attributes::Vector{JLD2.ReadAttribute})
    @ JLD2 ~/.julia/packages/JLD2/GojoK/src/datasets.jl:272
  [2] macro expansion
    @ ~/.julia/packages/JLD2/GojoK/src/datasets.jl:224 [inlined]
  [3] macro expansion
    @ ~/.julia/packages/JLD2/GojoK/src/datatypes.jl:105 [inlined]
  [4] read_data(f::JLD2.JLDFile{JLD2.MmapIO}, dataspace::JLD2.ReadDataspace, datatype_class::UInt8, datatype_offset::Int64, layout::JLD2.DataLayout, filters::JLD2.FilterPipeline, header_offset::JLD2.RelOffset, attributes::Vector{JLD2.ReadAttribute})
    @ JLD2 ~/.julia/packages/JLD2/GojoK/src/datasets.jl:211
  [5] load_dataset(f::JLD2.JLDFile{JLD2.MmapIO}, offset::JLD2.RelOffset)
    @ JLD2 ~/.julia/packages/JLD2/GojoK/src/datasets.jl:125
  [6] jlconvert
    @ ~/.julia/packages/JLD2/GojoK/src/data/writing_datatypes.jl:314 [inlined]
  [7] macro expansion
    @ ~/.julia/packages/JLD2/GojoK/src/data/reconstructing_datatypes.jl:574 [inlined]
  [8] jlconvert(#unused#::JLD2.ReadRepresentation{B, JLD2.OnDiskRepresentation{(0,), Tuple{Vector{A}}, Tuple{JLD2.RelOffset}, 8}()}, f::JLD2.JLDFile{JLD2.MmapIO}, ptr::Ptr{Nothing}, header_offset::JLD2.RelOffset)
    @ JLD2 ~/.julia/packages/JLD2/GojoK/src/data/reconstructing_datatypes.jl:539
  [9] read_scalar(f::JLD2.JLDFile{JLD2.MmapIO}, rr::JLD2.ReadRepresentation{B, JLD2.OnDiskRepresentation{(0,), Tuple{Vector{A}}, Tuple{JLD2.RelOffset}, 8}()}, header_offset::JLD2.RelOffset)
    @ JLD2 ~/.julia/packages/JLD2/GojoK/src/dataio.jl:37
 [10] read_data(f::JLD2.JLDFile{JLD2.MmapIO}, rr::Any, read_dataspace::Tuple{JLD2.ReadDataspace, JLD2.RelOffset, JLD2.DataLayout, JLD2.FilterPipeline}, attributes::Vector{JLD2.ReadAttribute})
    @ JLD2 ~/.julia/packages/JLD2/GojoK/src/datasets.jl:238
 [11] read_data(f::JLD2.JLDFile{JLD2.MmapIO}, dataspace::JLD2.ReadDataspace, datatype_class::UInt8, datatype_offset::Int64, layout::JLD2.DataLayout, filters::JLD2.FilterPipeline, header_offset::JLD2.RelOffset, attributes::Vector{JLD2.ReadAttribute})
    @ JLD2 ~/.julia/packages/JLD2/GojoK/src/datasets.jl:194
 [12] load_dataset(f::JLD2.JLDFile{JLD2.MmapIO}, offset::JLD2.RelOffset)
    @ JLD2 ~/.julia/packages/JLD2/GojoK/src/datasets.jl:125
 [13] getindex(g::JLD2.Group{JLD2.JLDFile{JLD2.MmapIO}}, name::String)
    @ JLD2 ~/.julia/packages/JLD2/GojoK/src/groups.jl:109
 [14] read
    @ ~/.julia/packages/JLD2/GojoK/src/JLD2.jl:456 [inlined]
 [15] (::JLD2.var"#103#104"{String})(file::JLD2.JLDFile{JLD2.MmapIO})
    @ JLD2 ~/.julia/packages/JLD2/GojoK/src/fileio.jl:46
 [16] jldopen(::Function, ::String, ::Vararg{String}; kws::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
    @ JLD2 ~/.julia/packages/JLD2/GojoK/src/loadsave.jl:4
 [17] #fileio_load#102
    @ ~/.julia/packages/JLD2/GojoK/src/fileio.jl:45 [inlined]
 [18] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
    @ Base ./essentials.jl:731
 [19] action(call::Symbol, libraries::Vector{Union{Base.PkgId, Module}}, file::FileIO.Formatted, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
    @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:219
 [20] action(call::Symbol, libraries::Vector{Union{Base.PkgId, Module}}, sym::Symbol, file::String, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
    @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:185
 [21] load(file::String, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
    @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:113
 [22] top-level scope
    @ REPL[8]:1
Stacktrace:
 [1] handle_error(e::TypeError, q::Base.PkgId, bt::Vector{Union{Ptr{Nothing}, Base.InterpreterIP}})
   @ FileIO ~/.julia/packages/FileIO/aP78L/src/error_handling.jl:61
 [2] handle_exceptions(exceptions::Vector{Tuple{Any, Union{Base.PkgId, Module}, Vector}}, action::String)
   @ FileIO ~/.julia/packages/FileIO/aP78L/src/error_handling.jl:56
 [3] action(call::Symbol, libraries::Vector{Union{Base.PkgId, Module}}, file::FileIO.Formatted, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
   @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:228
 [4] action(call::Symbol, libraries::Vector{Union{Base.PkgId, Module}}, sym::Symbol, file::String, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
   @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:185
 [5] load(file::String, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
   @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:113
 [6] top-level scope
   @ REPL[8]:1
JonasIsensee commented 1 year ago

Hi @dpinol,

I'm glad to hear that. Also, your ability to find bugs is truly incredible :joy: It should be fixed now. (The failure on x86 appears unrelated)

dpinol commented 1 year ago

I'm glad to hear that. Also, your ability to find bugs is truly incredible joy It should be fixed now. (The failure on x86 appears unrelated)

you're ability to fix them is also unmöglich :-)

I'm afraid I have another one if using Set instead of Vector

using JLD2

Base.@kwdef mutable struct A
    x::Int
    y::Float64
end

Base.@kwdef mutable struct B
    a::Set{A}
end

function JLD2.rconvert(::Type{A}, nt::NamedTuple)
    @info "loading from NamedTuple" nt
    A(; nt...)
end

@info "save"
save_object("test.jld2", B(Set([A(; x=1, y=2.0)])))

@info "load"
@info "loaded" load_object("test.jld2")

@info "patch"
loaded=JLD2.load("test.jld2", "single_stored_object"; typemap=Dict("Main.A" => JLD2.Upgrade(A)))
@info "patched" loaded

I get 

 Info: save
[ Info: load
┌ Info: loaded
└   load_object("test.jld2") = B(Set(A[A(1, 2.0)]))
[ Info: patch
┌ Warning: type parameters for Set{JLD2.Upgrade(A)} do not match type Set in workspace; reconstructing
└ @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/data/reconstructing_datatypes.jl:467
┌ Warning: type parameters for Pair{JLD2.Upgrade(A),Nothing} do not match type Pair in workspace; reconstructing
└ @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/data/reconstructing_datatypes.jl:467
┌ Warning: custom serialization of Dict{JLD2.Upgrade(A),Nothing} encountered, but the type does not exist in the workspace; the data will be read unconverted
└ @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/data/reconstructing_datatypes.jl:62
┌ Warning: saved type B has field a::JLD2.ReconstructedTypes.var"##Set{JLD2.Upgrade(A)}#313", but workspace type has field a::Set{A}, and no applicable convert method exists; reconstructing
└ @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/data/reconstructing_datatypes.jl:182
┌ Warning: type Pair{JLD2.Upgrade(A),Nothing} does not exist in workspace; interpreting Array{Pair{JLD2.Upgrade(A),Nothing}} as Array{Any}
└ @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/datasets.jl:268
┌ Info: loading from NamedTuple
└   nt = (x = 1, y = 2.0)
┌ Info: patched
└   loaded = JLD2.ReconstructedTypes.var"##B#314"(JLD2.ReconstructedTypes.var"##Set{JLD2.Upgrade(A)}#313"(Any[JLD2.ReconstructedTypes.var"##Pair{JLD2.Upgrade(A),Nothing}#312"(A(1, 2.0))]))
dpinol commented 1 year ago

Similar errors with Ref

using JLD2

Base.@kwdef mutable struct A
    x::Int
    y::Float64
end

Base.@kwdef mutable struct B
    a::Ref{A}
end

function JLD2.rconvert(::Type{A}, nt::NamedTuple)
    @info "loading from NamedTuple" nt
    A(; nt...)
end

@info "save"
save_object("test.jld2", B(Ref(A(; x=1, y=2.0))))

@info "load"
@info "loaded" load_object("test.jld2")

@info "patch"
loaded=JLD2.load("test.jld2", "single_stored_object"; typemap=Dict("Main.A" => JLD2.Upgrade(A)))
@info "patched" loaded

I get

[ Info: save
[ Info: load
┌ Info: loaded
└   load_object("test.jld2") = B(Base.RefValue{A}(A(1, 2.0)))
[ Info: patch
┌ Warning: type parameters for Base.RefValue{JLD2.Upgrade(A)} do not match type Base.RefValue in workspace; reconstructing
└ @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/data/reconstructing_datatypes.jl:467
┌ Info: loading from NamedTuple
└   nt = (x = 1, y = 2.0)
Error encountered while load FileIO.File{FileIO.DataFormat{:JLD2}, String}("test.jld2").

Fatal error:
ERROR: LoadError: MethodError: Cannot `convert` an object of type JLD2.ReconstructedTypes.var"##Base.RefValue{JLD2.Upgrade(A)}#312" to an object of type A
Closest candidates are:
  convert(::Type{T}, ::T) where T at Base.jl:61
  A(::Any, ::Any) at ~/dev/julia/sandboxes/jld2upgrade/run.jl:5
Stacktrace:
  [1] Base.RefValue{A}(x::JLD2.ReconstructedTypes.var"##Base.RefValue{JLD2.Upgrade(A)}#312")
    @ Base ./refvalue.jl:8
  [2] convert(#unused#::Type{Ref{A}}, x::JLD2.ReconstructedTypes.var"##Base.RefValue{JLD2.Upgrade(A)}#312")
    @ Base ./refpointer.jl:104
  [3] rconvert(T::Type, x::JLD2.ReconstructedTypes.var"##Base.RefValue{JLD2.Upgrade(A)}#312")
    @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/data/custom_serialization.jl:10
  [4] jlconvert
    @ ~/.julia/packages/JLD2/XpZ3T/src/data/writing_datatypes.jl:315 [inlined]
  [5] macro expansion
    @ ~/.julia/packages/JLD2/XpZ3T/src/data/reconstructing_datatypes.jl:574 [inlined]
  [6] jlconvert(#unused#::JLD2.ReadRepresentation{B, JLD2.OnDiskRepresentation{(0,), Tuple{Ref{A}}, Tuple{JLD2.RelOffset}, 8}()}, f::JLD2.JLDFile{JLD2.MmapIO}, ptr::Ptr{Nothing}, header_offset::JLD2.RelOffset)
    @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/data/reconstructing_datatypes.jl:539
  [7] read_scalar(f::JLD2.JLDFile{JLD2.MmapIO}, rr::JLD2.ReadRepresentation{B, JLD2.OnDiskRepresentation{(0,), Tuple{Ref{A}}, Tuple{JLD2.RelOffset}, 8}()}, header_offset::JLD2.RelOffset)
    @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/dataio.jl:37
  [8] read_data(f::JLD2.JLDFile{JLD2.MmapIO}, rr::Any, read_dataspace::Tuple{JLD2.ReadDataspace, JLD2.RelOffset, JLD2.DataLayout, JLD2.FilterPipeline}, attributes::Vector{JLD2.ReadAttribute})
    @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/datasets.jl:238
  [9] read_data(f::JLD2.JLDFile{JLD2.MmapIO}, dataspace::JLD2.ReadDataspace, datatype_class::UInt8, datatype_offset::Int64, layout::JLD2.DataLayout, filters::JLD2.FilterPipeline, header_offset::JLD2.RelOffset, attributes::Vector{JLD2.ReadAttribute})
    @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/datasets.jl:194
 [10] load_dataset(f::JLD2.JLDFile{JLD2.MmapIO}, offset::JLD2.RelOffset)
    @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/datasets.jl:125
 [11] getindex(g::JLD2.Group{JLD2.JLDFile{JLD2.MmapIO}}, name::String)
    @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/groups.jl:109
 [12] read
    @ ~/.julia/packages/JLD2/XpZ3T/src/JLD2.jl:456 [inlined]
 [13] (::JLD2.var"#103#104"{String})(file::JLD2.JLDFile{JLD2.MmapIO})
    @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/fileio.jl:46
 [14] jldopen(::Function, ::String, ::Vararg{String}; kws::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
    @ JLD2 ~/.julia/packages/JLD2/XpZ3T/src/loadsave.jl:4
 [15] #fileio_load#102
    @ ~/.julia/packages/JLD2/XpZ3T/src/fileio.jl:45 [inlined]
 [16] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
    @ Base ./essentials.jl:731
 [17] action(call::Symbol, libraries::Vector{Union{Base.PkgId, Module}}, file::FileIO.Formatted, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
    @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:219
 [18] action(call::Symbol, libraries::Vector{Union{Base.PkgId, Module}}, sym::Symbol, file::String, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
    @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:185
 [19] load(file::String, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
    @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:113
 [20] top-level scope
    @ ~/dev/julia/sandboxes/jld2upgrade/run.jl:25
Stacktrace:
 [1] handle_error(e::MethodError, q::Base.PkgId, bt::Vector{Union{Ptr{Nothing}, Base.InterpreterIP}})
   @ FileIO ~/.julia/packages/FileIO/aP78L/src/error_handling.jl:61
 [2] handle_exceptions(exceptions::Vector{Tuple{Any, Union{Base.PkgId, Module}, Vector}}, action::String)
   @ FileIO ~/.julia/packages/FileIO/aP78L/src/error_handling.jl:56
 [3] action(call::Symbol, libraries::Vector{Union{Base.PkgId, Module}}, file::FileIO.Formatted, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
   @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:228
 [4] action(call::Symbol, libraries::Vector{Union{Base.PkgId, Module}}, sym::Symbol, file::String, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
   @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:185
 [5] load(file::String, args::String; options::Base.Pairs{Symbol, Dict{String, JLD2.Upgrade}, Tuple{Symbol}, NamedTuple{(:typemap,), Tuple{Dict{String, JLD2.Upgrade}}}})
   @ FileIO ~/.julia/packages/FileIO/aP78L/src/loadsave.jl:113
 [6] top-level scope
   @ ~/dev/julia/sandboxes/jld2upgrade/run.jl:25
JonasIsensee commented 1 year ago

fixed :)

dpinol commented 1 year ago

awesome! The final one, duplicated references are repeated. Curiously rconvert is only called once

Base.@kwdef mutable struct A
    x::Int
    y::Float64
end

Base.@kwdef mutable struct B
    a::Vector{A}
end

function JLD2.rconvert(::Type{A}, nt::NamedTuple)
    @info "loading from NamedTuple" nt
    A(; nt...)
end

@info "save"
a = A(; x=1, y=2.0)
save_object("test.jld2", B([a, a]))

@info "load"
loaded = load_object("test.jld2")
@info "loaded" loaded
@assert loaded.a[1] === loaded.a[2]

@info "patch"
loaded = JLD2.load("test.jld2", "single_stored_object"; typemap=Dict("Main.A" => JLD2.Upgrade(A)))
@info "patched" loaded
@assert loaded.a[1] === loaded.a[2]
[ Info: save
[ Info: load
┌ Info: loaded
└   loaded = B(A[A(1, 2.0), A(1, 2.0)])
[ Info: patch
┌ Info: loading from NamedTuple
└   nt = (x = 1, y = 2.0)
┌ Info: patched
└   loaded = B(A[A(1, 2.0), A(1, 2.0)])
ERROR: LoadError: AssertionError: loaded.a[1] === loaded.a[2]
dpinol commented 1 year ago

And with that my whole project has fully automatic backwards compatible serialization! :place_of_worship: :place_of_worship: :place_of_worship:

dpinol commented 1 year ago

I have some feedback on the API. Imagine a struct is in its version 3. With the current API, I don't see a way to know if I need to convert the data from V1 or from V2.
This is because even if I change the struct name for each version, I understand that in the typemap Dict I always need to set the latest version of the struct to the Upgrade object. Hence, converting from V1 to V3, or from V2 to V3 will always invoke the same function. And from this function, I have no way to know I the named tuple was created from V1 or V2.

Possibilities:

However, since normally migrations add, remove or rename a field, I can always check the names of the NamedTuple fields, and hence I can live with this.

thanks again

JonasIsensee commented 1 year ago

I fear that adding this feature would complicate things considerably.

I would also argue that changing struct meanings without changing name or field names or field types would essentially be asking for problems..

This PR still requires converting your examples into CI tests, a small docs section, changelog and version bump.

dpinol commented 1 year ago

I would also argue that changing struct meanings without changing name or field names or field types would essentially be asking for problems..

yes, you're right. We're still on "prototype mode", but I agree that changing semantics is a bad idea.

This PR still requires converting your examples into CI tests, a small docs section, changelog and version bump.

no worries, thanks