JuliaArrays / BlockArrays.jl

BlockArrays for Julia
http://juliaarrays.github.io/BlockArrays.jl/
Other
194 stars 27 forks source link

Array conversion breaks image outputs? #415

Closed kescobo closed 1 week ago

kescobo commented 1 month ago

Blocks containing images work great, but there's a bug when we try to show the image as an image.

julia> using BlockArrays, Images

julia> img1 = rand(RGB, 3,2); img2 = rand(RGB, 3, 3);

julia> m = mortar([[img1] [img2]])
1×2-blocked 3×5 BlockMatrix{RGB{Float64}}:
 RGB{Float64}(0.571161,0.0966364,0.060899)  RGB{Float64}(0.422479,0.996909,0.792217)   │  …  RGB{Float64}(0.850701,0.105502,0.168711)  RGB{Float64}(0.32332,0.455819,0.829054)   │
 RGB{Float64}(0.0862087,0.399605,0.569387)  RGB{Float64}(0.0457857,0.330564,0.426887)  │     RGB{Float64}(0.605108,0.28189,0.40974)    RGB{Float64}(0.131377,0.939382,0.495051)  │
 RGB{Float64}(0.84549,0.349125,0.824663)    RGB{Float64}(0.891341,0.0830336,0.981568)  │     RGB{Float64}(0.153649,0.972397,0.891223)  RGB{Float64}(0.616976,0.453239,0.156959)  │

julia> io = IOBuffer()
IOBuffer(data=UInt8[...], readable=true, writable=true, seekable=true, append=false, size=0, maxsize=Inf, ptr=1, mark=-1)

julia> show(io, "image/png", m)
ERROR: MethodError: Cannot `convert` an object of type RGB{Float64} to an object of type Float64
The function `convert` exists, but no method is defined for this combination of argument types.

This came up because I'm using SixelTerm.jl, and these blocked images are throwing errors every time it tries to print to the REPL. @eschnett Seems to have traced it (see here for the full context) to

function Base.Array(block_array::BlockArray{T, N, R}) where {T,N,R}
    arr = Array{eltype(T)}(undef, size(block_array))
    for block_index in Iterators.product(blockaxes(block_array)...)
        indices = getindex.(axes(block_array), block_index)
        arr[indices...] = @view block_array[block_index...]
    end
    return arr
end

The expression eltype(T) converts T – which is already the array's element type – to the element type of T. For scalars this is a no-op, for RGB{Float64} this is apparently Float64. I think it would suffice to replace eltype(T) just by T.

Xref #413 and https://github.com/eschnett/SixelTerm.jl/issues/13

dlfivefifty commented 1 month ago

Yes I think you are right can you make a PR?