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

Upgrading old structures with parametric composite types #474

Closed
dcerkoney opened this issue Jul 18, 2023 · 5 comments
Closed

Upgrading old structures with parametric composite types #474

dcerkoney opened this issue Jul 18, 2023 · 5 comments

Comments

@dcerkoney
Copy link

Hello,

I am encountering a small issue related to upgrading old structures on load using JLD2 v0.4.32 which I am struggling to debug.

Are parametric composite types supported for JLD2.rconvert and JLD2.Upgrade?
If so, how should the type mapping be specified?

Here is a small example illustrating the behavior I am expecting, which does not work as intended:

using JLD2
using Test

struct OldStruct{T}
    x::T
end

jldsave("test.jld2"; data=OldStruct{Float64}(0))

struct NewStruct{T}
    x::T
    b::Bool
end

function JLD2.rconvert(::Type{NewStruct{T}}, nt::NamedTuple) where {T}
    return NewStruct{T}(nt.x, true)
end

@test JLD2.rconvert(NewStruct{Float64}, (x=0.0,)) == NewStruct{Float64}(0, true)

d = load(
    "test.jld2",
    "data";
    typemap=Dict("Main.OldStruct{Float64}" => JLD2.Upgrade(NewStruct{Float64}))
)

@test_broken d isa NewStruct{Float64}
@test_broken !(d isa OldStruct{Float64})

If there is a way to achieve something like this, it would be great if we could add an example to the docs.

Thanks,
Daniel

@JonasIsensee
Copy link
Collaborator

Hi @dcerkoney,

this should be possible with (note, that I removed the type parameter from the OldStruct)

d = load(
    "test.jld2",
    "data";
    typemap=Dict("Main.OldStruct" => JLD2.Upgrade(NewStruct{Float64}))
)

However, because of a bug, this still doesn't work. I think I fixed it in #475
#475

@dcerkoney
Copy link
Author

Great, thanks for the swift response!

@dcerkoney
Copy link
Author

One problem comes to mind with the proposed solution: doesn't it mean that an archive holding different specializations of OldStruct{T} cannot be straightforwardly upgraded to hold NewStruct{T} objects?

For example, suppose we have a situation like this:

using JLD2

struct OldStruct{T}
    x::T
end

struct NewStruct{T}
    x::T
    b::Bool
end

function JLD2.rconvert(::Type{NewStruct{T}}, nt::NamedTuple) where {T}
    return NewStruct{T}(nt.x, true)
end

jldsave(
    "test.jld2";
    a=OldStruct{Int}(1),
    b=OldStruct{Vector{Int}}([1, 0]),
    c=OldStruct{Matrix{Int}}([1 0; 0 1])
)

Since I have defined rconvert for all T, I would then hope to see one or both of the following options:

# Option 1: Try to upgrade every OldStruct{T} to NewStruct{T} where {T}
d1 = load(
    "test.jld2";
    typemap=Dict("Main.OldStruct" => JLD2.Upgrade(NewStruct))
)

# Option 2: Upgrade OldStruct{T} to NewStruct{T} for explicitly specified values of T
d2 = load(
    "test.jld2";
    typemap=Dict(
        "Main.OldStruct{Vector{Int}}" => JLD2.Upgrade(NewStruct{Vector{Int}}),
        "Main.OldStruct{Matrix{Int}}" => JLD2.Upgrade(NewStruct{Matrix{Int}}),
    )
)

Any thoughts?

@JonasIsensee
Copy link
Collaborator

Would it work to define a single upgrade statement that does "Main.OldStruct" => Upgrade(NewStruct)

@dcerkoney
Copy link
Author

Sure, that approach makes sense to me.

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

No branches or pull requests

2 participants