JuliaIO / JLD2.jl

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

array of NamedTuple from julia 1.6 to julia 1.7 fail to load #393

Closed anj00 closed 11 months ago

anj00 commented 2 years ago

A file written in julia 1.6.5 with JLD2 v0.4.19 save("tmp.jld2", Dict("tmp" => [(x1 = 1, x2 = 2), (x1 = 1, x2 = 2, x3 = 3)])) Note above: the tuples have different number of fields

Fails to be loaded in julia 1.7.2 with JLD2 v0.4.22 which obviously makes it hard to migrate. Note, an array of identical NamedTuple is fine. It is only then one adds/removes fields in the NamedTuple in the array the problem shows up.

The error

Fatal error:
ERROR: MethodError: Cannot `convert` an object of type 
  NamedTuple{(:x1, :x2), Tuple{Int64, Int64}} to an object of type
  NamedTuple{names, T} where {names, T<:Tuple{JLD2.UnknownType{String}("Core.Vararg", Any[Any, N])}}
Closest candidates are:
  convert(::Type{T}, ::T) where T at C:\Bin\Julia-1.7.2\share\julia\base\essentials.jl:218
Stacktrace:
 [1] rconvert(T::Type, x::NamedTuple{(:x1, :x2), Tuple{Int64, Int64}})
   @ JLD2 C:\Users\win_user\.julia\packages\JLD2\k9Gt0\src\data\custom_serialization.jl:10
 [2] jlconvert
   @ C:\Users\win_user\.julia\packages\JLD2\k9Gt0\src\data\writing_datatypes.jl:305 [inlined]
 [3] macro expansion
   @ C:\Users\win_user\.julia\packages\JLD2\k9Gt0\src\dataio.jl:70 [inlined]
 [4] macro expansion
   @ .\simdloop.jl:77 [inlined]
 [5] read_array!(v::Vector{NamedTuple{names, T} where {names, T<:Tuple{JLD2.UnknownType{String}("Core.Vararg", Any[Any, N])}}}, f::JLD2.JLDFile{JLD2.MmapIO}, rr::JLD2.ReadRepresentation{NamedTuple{names, T} where {names, T<:Tuple{JLD2.UnknownType{String}("Core.Vararg", Any[Any, N])}}, JLD2.RelOffset})
   @ JLD2 C:\Users\win_user\.julia\packages\JLD2\k9Gt0\src\dataio.jl:68
 [6] read_array(f::JLD2.JLDFile{JLD2.MmapIO}, dataspace::JLD2.ReadDataspace, rr::JLD2.ReadRepresentation{NamedTuple{names, T} where {names, T<:Tuple{JLD2.UnknownType{String}("Core.Vararg", Any[Any, N])}}, JLD2.RelOffset}, data_length::Int64, filter_id::UInt16, header_offset::JLD2.RelOffset, attributes::Vector{JLD2.ReadAttribute})
   @ JLD2 C:\Users\win_user\.julia\packages\JLD2\k9Gt0\src\datasets.jl:329
 [7] read_data(f::JLD2.JLDFile{JLD2.MmapIO}, rr::JLD2.ReadRepresentation{Any, JLD2.RelOffset}, read_dataspace::Tuple{JLD2.ReadDataspace, JLD2.RelOffset, Int64, UInt16}, attributes::Vector{JLD2.ReadAttribute})
   @ JLD2 C:\Users\win_user\.julia\packages\JLD2\k9Gt0\src\datasets.jl:200
Stacktrace:
 [1] handle_error(e::MethodError, q::Base.PkgId, bt::Vector{Union{Ptr{Nothing}, Base.InterpreterIP}})
   @ FileIO C:\Users\win_user\.julia\packages\FileIO\u9YLx\src\error_handling.jl:61
 [2] handle_exceptions(exceptions::Vector{Tuple{Any, Union{Base.PkgId, Module}, Vector}}, action::String)
   @ FileIO C:\Users\win_user\.julia\packages\FileIO\u9YLx\src\error_handling.jl:56
 [3] action(call::Symbol, libraries::Vector{Union{Base.PkgId, Module}}, file::Formatted, args::String; options::Base.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:compress,), Tuple{Bool}}})
   @ FileIO C:\Users\win_user\.julia\packages\FileIO\u9YLx\src\loadsave.jl:228
 [4] action(call::Symbol, libraries::Vector{Union{Base.PkgId, Module}}, sym::Symbol, file::String, args::String; options::Base.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:compress,), Tuple{Bool}}})
   @ FileIO C:\Users\win_user\.julia\packages\FileIO\u9YLx\src\loadsave.jl:185
 [5] load(file::String, args::String; options::Base.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:compress,), Tuple{Bool}}})
   @ FileIO C:\Users\win_user\.julia\packages\FileIO\u9YLx\src\loadsave.jl:113
JonasIsensee commented 2 years ago

Hi @anj00 , thank you for reporting this. It is definitely a bug and I will try to fix it soon.

JonasIsensee commented 2 years ago

I have tracked down the issue to this: julia internals changed between 1.6 and 1.7 on 1.6

julia> Vararg{Int, 3} isa DataType
true

while on 1.7

julia> Vararg{Int,3} isa DataType
false
JonasIsensee commented 2 years ago

Hi @anj00 ,

I think, I implemented a working fix for you in #394 . However, the changes are rather ugly and require extra checks that will be executed many many times, so I'm a bit hesitant to merge it. You're welcome to try it out, though.

anj00 commented 2 years ago

Thanks! varargfix branch loads fine both the test file and the real data I had issues with.