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

Array conversion breaks image outputs? #415

Closed
kescobo opened this issue Aug 7, 2024 · 1 comment · Fixed by #416
Closed

Array conversion breaks image outputs? #415

kescobo opened this issue Aug 7, 2024 · 1 comment · Fixed by #416

Comments

@kescobo
Copy link
Contributor

kescobo commented Aug 7, 2024

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 eschnett/SixelTerm.jl#13

@dlfivefifty
Copy link
Member

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

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

Successfully merging a pull request may close this issue.

2 participants