JuliaIO / BSON.jl

Other
158 stars 39 forks source link

Array type not preserved when loading. Saved as `Array{String,1}` but loaded as `Array{Any,1}`. #48

Open DilumAluthge opened 5 years ago

DilumAluthge commented 5 years ago
julia> using BSON
julia> x = ["foo", "bar", "baz"]
3-element Array{String,1}:
 "foo"
 "bar"
 "baz"
julia> typeof(x)
Array{String,1}
julia> bson("test.bson", Dict(:x => x))

Now close Julia and open a new Julia session.

julia> using BSON

julia> data = BSON.load("test.bson")
Dict{Symbol,Any} with 1 entry:
  :x => Any["foo", "bar", "baz"]

julia> data[:x]
3-element Array{Any,1}:
 "foo"
 "bar"
 "baz"

julia> typeof(data[:x])
Array{Any,1}
DilumAluthge commented 5 years ago

Here is some code that recursively fixes the types of arrays.

function _assigned(a::AbstractArray{T, N})::BitArray{N} where T where N
    result::BitArray{N} = BitArray{N}(undef, size(a))
    for i = 1:length(a)
        result[i] = isassigned(a, i)
    end
    return result
end

_correct_typeof(x::T) where T <: Any = T

_correct_typeof(a::A) where A <: AbstractArray{T, N} where T where N = Array{_correct_eltype(a), N}

_correct_eltype(a::A) where A <: AbstractArray{T, N} where T where N = _bottom_to_any(typejoin(_correct_typeof.(a[_assigned(a)])...))

_bottom_to_any(x) = x
_bottom_to_any(::Core.TypeofBottom) = Any

fixtype(x) = x
function fixtype(a::A) where A <: AbstractArray{T, N} where T where N
    S = _correct_typeof(a)
    new_array::S = S(undef, size(a))
    for i = 1:length(a)
        if isassigned(a, i)
            new_array[i] = fixtype(a[i])
        end
    end
    return new_array
end
DilumAluthge commented 5 years ago

The usage is fixtype(a).

Example:

julia> x = Any["foo", "bar", "baz"]
3-element Array{Any,1}:
 "foo"
 "bar"
 "baz"

julia> typeof(x)
Array{Any,1}

julia> y = fixtype(x)
3-element Array{String,1}:
 "foo"
 "bar"
 "baz"

julia> typeof(y)
Array{String,1}
DilumAluthge commented 5 years ago

@MikeInnes Should we add this code to BSON.jl and call fixtype(a) on every Array that we load? Or should we simply inform the user that the Array eltype they saved is not going to be the same Array eltype that they loaded, and then let the user take care of fixing the eltypes themselves?

MikeInnes commented 5 years ago

Honestly I think this is just a design flaw of BSON.jl in its current form, which tries to simulataneously be convenient for raw BSON data structures and for Julia ones.

Ideally, what I'd like to do is split this package into a raw BSON reader and a Julia serialiser on top of that. The serialise should always respect eltypes like this.

Note that something like fixtype isn't an ideal solution since someone might save an array with an abstract type parameter, for example, and then we're still not able to round trip it.

DrChainsaw commented 4 years ago

Hi,

I ran into this issue with some structs that had fields types like arr::AbstractArray{<:Integer} which could not be deserialized because of this.

The following hackaround did the job for me:

function BSON.newstruct!(x, fs...)
  for (i, f) = enumerate(fs)
    f = fixarrtype(f)
    f = convert(fieldtype(typeof(x),i), f)
    ccall(:jl_set_nth_field, Nothing, (Any, Csize_t, Any), x, i-1, f)
  end
  return x
end
fixarrtype(f) = f
fixarrtype(f::AbstractVector{Any}) = map(fixarrtype, f)
rlrs commented 3 years ago

I think I'm having an issue related to this when saving/loading a struct containing arrays:

ERROR: MethodError: no method matching Array{var"#s20",1} where var"#s20"<:Typ(::Array{Any,1}) Stacktrace: [1] convert(::Type{Array{var"#s20",1} where var"#s20"<:Typ}, ::Array{Any,1}) at ./array.jl:554 [2] newstruct(::Type{T} where T, ::String, ::Vararg{Any,N} where N) at /home/ralars/.julia/packages/BSON/XAts7/src/extensions.jl:106 [3] (::BSON.var"#41#42")(::Dict{Symbol,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/extensions.jl:130 [4] _raise_recursive(::Dict{Symbol,Any}, ::IdDict{Any,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:79 [5] (::BSON.var"#43#44")(::Dict{Symbol,Any}, ::IdDict{Any,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/extensions.jl:139 [6] raise_recursive(::Dict{Symbol,Any}, ::IdDict{Any,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:88 [7] (::BSON.var"#21#22"{IdDict{Any,Any}})(::Dict{Symbol,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:94 [8] applychildren!(::BSON.var"#21#22"{IdDict{Any,Any}}, ::Array{Any,1}) at /home/ralars/.julia/packages/BSON/XAts7/src/BSON.jl:28 [9] raise_recursive at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:94 [inlined] ... (the last 3 lines are repeated 1 more time) [13] (::BSON.var"#17#19"{IdDict{Any,Any}})(::Array{Any,1}) at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:79 [14] applychildren!(::BSON.var"#17#19"{IdDict{Any,Any}}, ::Dict{Symbol,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/BSON.jl:21 [15] _raise_recursive(::Dict{Symbol,Any}, ::IdDict{Any,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:79 [16] (::BSON.var"#43#44")(::Dict{Symbol,Any}, ::IdDict{Any,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/extensions.jl:139 [17] raise_recursive(::Dict{Symbol,Any}, ::IdDict{Any,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:88 ... (the last 8 lines are repeated 1 more time) [26] (::BSON.var"#39#40"{IdDict{Any,Any}})(::Dict{Symbol,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/extensions.jl:124 [27] iterate at ./generator.jl:47 [inlined] [28] _collect(::Array{Any,1}, ::Base.Generator{Array{Any,1},BSON.var"#39#40"{IdDict{Any,Any}}}, ::Base.EltypeUnknown, ::Base.HasShape{1}) at ./array.jl:699 [29] collect_similar(::Array{Any,1}, ::Base.Generator{Array{Any,1},BSON.var"#39#40"{IdDict{Any,Any}}}) at ./array.jl:628 [30] map(::Function, ::Array{Any,1}) at ./abstractarray.jl:2162 [31] newstruct_raw(::IdDict{Any,Any}, ::Type{T} where T, ::Dict{Symbol,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/extensions.jl:124 [32] (::BSON.var"#43#44")(::Dict{Symbol,Any}, ::IdDict{Any,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/extensions.jl:140 [33] raise_recursive(::Dict{Symbol,Any}, ::IdDict{Any,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:88 ... (the last 8 lines are repeated 1 more time) [42] (::BSON.var"#18#20"{IdDict{Any,Any}})(::Dict{Symbol,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:82 [43] applychildren!(::BSON.var"#18#20"{IdDict{Any,Any}}, ::Dict{Symbol,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/BSON.jl:21 [44] _raise_recursive(::Dict{Symbol,Any}, ::IdDict{Any,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:82 [45] raise_recursive(::Dict{Symbol,Any}, ::IdDict{Any,Any}) at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:89 [46] raise_recursive at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:99 [inlined] [47] load(::String) at /home/ralars/.julia/packages/BSON/XAts7/src/read.jl:104 [48] top-level scope at /home/ralars/.julia/packages/BSON/XAts7/src/BSON.jl:52 [49] include(::String) at ./client.jl:457

BoZenKhaa commented 3 years ago

I ran into this with an array of MyStruct. My hacky solution was to repack the loaded array with new = [old...] and let Julia decide on the array type itself.

If there is no plan to address this in the current package, I think it would be worthwhile to mention this in README as this behavior is unexpected and has the potential of intruducing unexpected type errors in the code using loaded data.