Multi-union wrapping

[matthijscox]

Another possible edge case that I wonder if it's easy to support in your package.

Let's say I want to wrap two unions at once in one type:

@wrapped struct Wrap
   a::Union{String,Int}
   b::Union{Float64, Float32}
end
f(w::Wrap) = @unionsplit f(w.a, w.b)

Right now I would have to do something like this I think:

struct Two{A,B}
   a::A
   b::B
end
f(t::Two) = f(t.a, t.b) # can this introduce dynamic dispatch?

const AllCombos = Union{
   Two{String,Float64}
   Two{String,Float32}
   Two{Int,Float64}
   Two{Int,Float32}
}
@wrapped struct Wrap
   union::AllCombos
end
f(w::Wrap) = @unionsplit f(w)

Would it be difficult to make the field union the default, but allow users to somehow overrule it by using the dot syntax w.a and w.b to choose which field to wrap?

[matthijscox]

Okay I realize I can do something like this, perhaps it's good enough (unless someone really wants to have the previous pattern):

using WrappedUnions

pairscore(::String, ::Float64) = 1.0
pairscore(::String, ::Float32) = 2.0
pairscore(::Int, ::Float64) = 3.0
pairscore(::Int, ::Float32) = 4.0

@wrapped struct Wrap1
   union::Union{String,Int}
end

@wrapped struct Wrap2
   union::Union{Float64, Float32}
end

# optional: parametric instead of `struct PairWrap; a::Wrap1; b::Wrap2; end`
struct PairWrap{A,B}
   a::A
   b::B
end
pairscore(t::PairWrap) = pairscore(t.a, t.b)
pairscore(t::PairWrap{Wrap1,Wrap2}) = @unionsplit pairscore(t.a, t.b)

I think my previous pattern is difficult to support with @wrapped given the hardcoded union field, which does make me wonder if you would be willing to support non-wrapped unionsplitting in this package, so we can have one unified unionsplitting package (with or without wrapping). But I'll make another issue for that.

Feel free to close this issue if you agree any attempt to enable this wrapping is impossible/nonsensical:

@wrapped struct Wrap
   a::Union{String,Int}
   b::Union{Float64, Float32}
end

[matthijscox]

Okay sorry for rambling, but I do now think my idea of non-wrapped union support is indeed not compatible with the whole philosophy of wrapped types that get dispatched in unionsplit(f, args). I tried to support raw unions in a branch and got type instability:

struct PairWrap
    a::Union{String, Int}
    b::Union{Float64, Float32}
end

pairscore(a::String, b::Float64) = 1
pairscore(a::String, b::Float32) = 2
pairscore(a::Int, b::Float64) = 3
pairscore(a::Int, b::Float32) = 4
pairscore(w::PairWrap) = @unionsplit pairscore(w.a, w.b)

julia> @report_opt pairscore(PairWrap("x", 1.0))
═════ 1 possible error found ═════
┌ pairscore(w::PairWrap) @ Main c:\Localdata\Repositories\WrappedUnions.jl\test\runtests.jl:127
│ runtime dispatch detected: unionsplit(pairscore, %3::Tuple{Union{Int64, String}, Union{Float32, Float64}}, [quote]::@NamedTuple{})::Any

If I ever want to support unwrapped unions, I'll just need another macro for like I attempted in UnionSplit.jl.

[ameligrana]

Hey! I think that indeed using two wrapped unions and another type to enclose those is how I originally thought this multi-union wrapping could be solved without supporting the multi-argument case explicitly. At the moment, I don't see a clear motivation to allow multiple fields, but if you can think of something which can't be done by creating multiple wrapped types and then enclosing them, maybe we can reconsider

[ameligrana]

I think my previous pattern is difficult to support with @wrapped given the hardcoded union field

what do you see as problematic by using the second pattern you came up with? The one with Wrap1 and Wrap2

[ameligrana]

Actually I think that it should be possible to support

struct PairWrap
    a::Union{String, Int}
    b::Union{Float64, Float32}
end

I mean the @wrapped macro doesn't do anything too fancy, it is actually more for validation than anything else, i.e. it validates that the first field is called union, so I think that by generalizing the generated function which create the branches it should be possible to have this...but I'm still unsure if just using multiple wrappers is not enough

[matthijscox]

I agree using two wrappers is probably sufficient so this is not high priority. I just like my freedom of choice.

If we ever decide to go with it, I suppose we need something like:

@wrapped (:a, :b) struct PairWrap
    a::Union{String, Int}
    b::Union{Float64, Float32}
    c::Real
end

Which then provides a function wrappedfields(::Type{PairWrap}) = (:a,:b) or something, which gets checked in unionsplit instead of the hardcoded __FIELDNAME__

We'd also have to infer which field belongs to which argument in unionsplit. So yeah, quite some work for little value.

[ameligrana]

Would make sense to me to make the change in the end to me, just as you say for freedom of choices, if you want to put up a PR doing this, actually maybe with this slight modification:

@wrapped (a, b) struct PairWrap
    a::Union{String, Int}
    b::Union{Float64, Float32}
    c::Real
end

[ameligrana]

At that point, though, I would also lift the name of the field to be union, just letting the user decide maybe.

The only thing to decide is in the default case, i.e.

@wrapped struct PairWrap
    a::Union{String, Int}
    b::Union{Float64, Float32}
    c::Real
end

what would the macro do. Maybe split all Union arguments?