Add missing stream kwargs & functionality for MIME #7

IanButterworth · 2020-08-19T17:23:05Z

Candidate fix for #6

@fonsp can you check whether this fixes your issue?

codecov · 2020-08-19T17:47:53Z

Codecov Report

Merging #7 into master will decrease coverage by 2.00%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master       #7      +/-   ##
==========================================
- Coverage   50.00%   48.00%   -2.00%     
==========================================
  Files           1        1              
  Lines          20       25       +5     
==========================================
+ Hits           10       12       +2     
- Misses         10       13       +3

Impacted Files	Coverage Δ
src/ImageIO.jl	`48.00% <60.00%> (-2.00%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f07b422...1bfe5b5. Read the comment docs.

fonsp · 2020-08-19T18:18:37Z

(@v1.5) pkg> activate asdfasdf

(asdfasdf) pkg> add Images https://github.com/JuliaIO/ImageIO.jl#ib/fix_mime

julia> using Images

julia> i = Gray.(rand(4,4))

julia> show(stdout, MIME("image/png"), i)
[ Info: Precompiling ImageIO [82e4d734-157c-48bb-816b-45c225c6df19]
Errors encountered while saving nothing.
All errors:
===========================================
MethodError: no method matching permutedims!(::Array{Gray{Float64},2}, ::Tuple{Int64,Int64})
Closest candidates are:
  permutedims!(::Array{T,N}, ::StridedArray{T, N}, ::Any) where {T, N} at multidimensional.jl:1394
  permutedims!(::Any, ::AbstractArray, ::Any) at permuteddimsarray.jl:194
===========================================
ArgumentError: Package ImageMagick not found in current path:
- Run `import Pkg; Pkg.add("ImageMagick")` to install the ImageMagick package.

===========================================
MethodError: no method matching save(::FileIO.Stream{FileIO.DataFormat{:PNG},Base.TTY}, ::Array{Gray{Float64},2}; mapi=ImageShow.var"#14#16"())
Closest candidates are:
  save(::FileIO.File{FileIO.DataFormat{:PNG}}, ::Any) at /home/fons/.julia/packages/FileIO/DzfYr/src/mimesave.jl:5 got unsupported keyword argument "mapi"
  save(::FileIO.File{FileIO.DataFormat{:SVG}}, ::Any) at /home/fons/.julia/packages/FileIO/DzfYr/src/mimesave.jl:15 got unsupported keyword argument "mapi"
  save(::FileIO.File{FileIO.DataFormat{:PDF}}, ::Any) at /home/fons/.julia/packages/FileIO/DzfYr/src/mimesave.jl:25 got unsupported keyword argument "mapi"
  ...
===========================================

Fatal error:
ERROR: MethodError: no method matching permutedims!(::Array{Gray{Float64},2}, ::Tuple{Int64,Int64})
Closest candidates are:
  permutedims!(::Array{T,N}, ::StridedArray{T, N}, ::Any) where {T, N} at multidimensional.jl:1394
  permutedims!(::Any, ::AbstractArray, ::Any) at permuteddimsarray.jl:194
Stacktrace:
 [1] handle_error(::MethodError, ::FileIO.Stream{FileIO.DataFormat{:PNG},Base.TTY}) at /home/fons/.julia/packages/FileIO/DzfYr/src/error_handling.jl:82
 [2] handle_exceptions(::Array{Any,1}, ::String) at /home/fons/.julia/packages/FileIO/DzfYr/src/error_handling.jl:77
 [3] save(::FileIO.Formatted, ::Any; options::Base.Iterators.Pairs{Symbol,ImageShow.var"#14#16",Tuple{Symbol},NamedTuple{(:mapi,),Tuple{ImageShow.var"#14#16"}}}) at /home/fons/.julia/packages/FileIO/DzfYr/src/loadsave.jl:217 [4] show(::Base.TTY, ::MIME{Symbol("image/png")}, ::Array{Gray{Float64},2}; minpixels::Int64, maxpixels::Int64, mapi::Function) at /home/fons/.julia/packages/ImageShow/9kpaq/src/showmime.jl:43
 [5] show(::Base.TTY, ::MIME{Symbol("image/png")}, ::Array{Gray{Float64},2}) at /home/fons/.julia/packages/ImageShow/9kpaq/src/showmime.jl:28
 [6] top-level scope at REPL[8]:1

fonsp · 2020-08-19T18:26:55Z

But it's not supposed to work if only Images and ImageIO are installed right?

IanButterworth · 2020-08-19T18:55:46Z

Ah ok.. I actually don't know whether we need the permute for the IJulia/Pluto use-case.. Would you mind trying again?

fonsp · 2020-08-19T19:12:24Z

Same setup as before, output:

julia> show(stdout, MIME("image/png"), i)

julia> repr(MIME("image/png"), i)
Errors encountered while saving nothing.
All errors:
===========================================
type GenericIOBuffer has no field lock
===========================================
ArgumentError: Package ImageMagick not found in current path:
- Run `import Pkg; Pkg.add("ImageMagick")` to install the ImageMagick package.

===========================================
MethodError: no method matching save(::FileIO.Stream{FileIO.DataFormat{:PNG},Base.GenericIOBuffer{Array{UInt8,1}}}, ::Array{Gray{Float64},2}; mapi=ImageShow.var"#14#16"())
Closest candidates are:
  save(::FileIO.File{FileIO.DataFormat{:PNG}}, ::Any) at /home/fons/.julia/packages/FileIO/DzfYr/src/mimesave.jl:5 got unsupported keyword argument "mapi"
  save(::FileIO.File{FileIO.DataFormat{:SVG}}, ::Any) at /home/fons/.julia/packages/FileIO/DzfYr/src/mimesave.jl:15 got unsupported keyword argument "mapi"
  save(::FileIO.File{FileIO.DataFormat{:PDF}}, ::Any) at /home/fons/.julia/packages/FileIO/DzfYr/src/mimesave.jl:25 got unsupported keyword argument "mapi"
  ...
===========================================

Fatal error:
ERROR: type GenericIOBuffer has no field lock
Stacktrace:
 [1] handle_error(::ErrorException, ::FileIO.Stream{FileIO.DataFormat{:PNG},Base.GenericIOBuffer{Array{UInt8,1}}}) at /home/fons/.julia/packages/FileIO/DzfYr/src/error_handling.jl:82
 [2] handle_exceptions(::Array{Any,1}, ::String) at /home/fons/.julia/packages/FileIO/DzfYr/src/error_handling.jl:77
 [3] save(::FileIO.Formatted, ::Any; options::Base.Iterators.Pairs{Symbol,ImageShow.var"#14#16",Tuple{Symbol},NamedTuple{(:mapi,),Tuple{ImageShow.var"#14#16"}}}) at /home/fons/.julia/packages/FileIO/DzfYr/src/loadsave.jl:217
 [4] show(::Base.GenericIOBuffer{Array{UInt8,1}}, ::MIME{Symbol("image/png")}, ::Array{Gray{Float64},2}; minpixels::Int64, maxpixels::Int64, mapi::Function) at /home/fons/.julia/packages/ImageShow/9kpaq/src/showmime.jl:43
 [5] show at /home/fons/.julia/packages/ImageShow/9kpaq/src/showmime.jl:28 [inlined]
 [6] __binrepr(::MIME{Symbol("image/png")}, ::Array{Gray{Float64},2}, ::Nothing) at ./multimedia.jl:159
 [7] _binrepr at ./multimedia.jl:0 [inlined]
 [8] #repr#1 at ./multimedia.jl:147 [inlined]
 [9] repr(::MIME{Symbol("image/png")}, ::Array{Gray{Float64},2}) at ./multimedia.jl:147
 [10] top-level scope at REPL[5]:1

julia> stdout.lock
ReentrantLock(nothing, Base.GenericCondition{Base.Threads.SpinLock}(Base.InvasiveLinkedList{Task}(nothing, nothing), Base.Threads.SpinLock(0)), 0)

Note that show(stdout, ...) did not write anything to stdout - in the old version of ImageIO, with ImageMagick installed, it does print (gibberish) to stdout.

When calling show on an IOBuffer instead of stdout using repr (as is the case in Pluto's display system), I get an error.

IanButterworth · 2020-08-19T19:14:53Z

I'm out of my depth here. Not worked with MIME etc before. If you see how to fix this, I think that might be the fastest way to a fix.

fonsp · 2020-08-19T19:16:33Z

Thanks for your help! I'm a little scared to jump into ImageIO's internals but I will keep it in mind.

IanButterworth · 2020-08-19T19:18:28Z

Perhaps @johnnychen94 or @SimonDanisch might be able to give guidance on a fix?

fonsp · 2020-08-19T19:23:09Z

(also see JuliaImages/ImageShow.jl#25)

SimonDanisch · 2020-08-19T19:42:00Z

maybe: JuliaIO/PNGFiles.jl#19

fonsp · 2020-08-19T19:43:34Z

I'd like to write a test case - in which repository do I do that?

johnnychen94

I don't have a clear view on how to fix the issue raised by @fonsp yet, probably I can take a deeper look this weekend. (We are busy at hosting Juliacon for CN community this week :D)

We can also add a short-circuit permute_horizontal=false && mapi == identity case to avoid memory allocation. And for cases that we must make a copy, I've made suggestions as comments.

src/ImageIO.jl

IanButterworth · 2020-08-20T17:06:58Z

@fonsp can you try this PR again with PNGFiles master (given JuliaIO/PNGFiles.jl#19 is now merged)

IanButterworth · 2020-08-26T03:37:04Z

@fonsp just a friendly ping

johnnychen94 · 2020-12-04T03:38:41Z

I'll rebase this to collaborate with JuliaIO/PNGFiles.jl#27

johnnychen94 · 2020-12-04T07:28:41Z

I've rebased this in #15 so close this one.

IanButterworth added 2 commits August 19, 2020 13:21

provide missing stream kwargs

de2818b

correct permute behavior

ae4dc2b

IanButterworth mentioned this pull request Aug 19, 2020

Interest in a standalone image/bmp method? JuliaImages/ImageShow.jl#25

Open

disable permute by default

cc54bda

fix permutedims!

00dc3fa

johnnychen94 reviewed Aug 20, 2020

View reviewed changes

src/ImageIO.jl Outdated Show resolved Hide resolved

src/ImageIO.jl Outdated Show resolved Hide resolved

suggested improvements

1bfe5b5

johnnychen94 mentioned this pull request Sep 3, 2020

Error loading images in VSCode REPL JuliaImages/Images.jl#912

Closed

johnnychen94 mentioned this pull request Sep 10, 2020

IOBuffer support JuliaIO/PNGFiles.jl#22

Closed

3 tasks

johnnychen94 mentioned this pull request Dec 4, 2020

Save to iobuffer JuliaIO/PNGFiles.jl#27

Merged

johnnychen94 mentioned this pull request Dec 4, 2020

Add missing stream kwargs & functionality for MIME #15

Merged

johnnychen94 closed this Dec 4, 2020

johnnychen94 deleted the ib/fix_mime branch December 4, 2020 07:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add missing stream kwargs & functionality for MIME #7

Add missing stream kwargs & functionality for MIME #7

IanButterworth commented Aug 19, 2020

codecov bot commented Aug 19, 2020 •

edited

Loading

fonsp commented Aug 19, 2020

fonsp commented Aug 19, 2020

IanButterworth commented Aug 19, 2020 •

edited

Loading

fonsp commented Aug 19, 2020 •

edited

Loading

IanButterworth commented Aug 19, 2020

fonsp commented Aug 19, 2020

IanButterworth commented Aug 19, 2020

fonsp commented Aug 19, 2020

SimonDanisch commented Aug 19, 2020

fonsp commented Aug 19, 2020

johnnychen94 left a comment •

edited

Loading

IanButterworth commented Aug 20, 2020

IanButterworth commented Aug 26, 2020

johnnychen94 commented Dec 4, 2020

johnnychen94 commented Dec 4, 2020

Add missing stream kwargs & functionality for MIME #7

Add missing stream kwargs & functionality for MIME #7

Conversation

IanButterworth commented Aug 19, 2020

codecov bot commented Aug 19, 2020 • edited Loading

Codecov Report

fonsp commented Aug 19, 2020

fonsp commented Aug 19, 2020

IanButterworth commented Aug 19, 2020 • edited Loading

fonsp commented Aug 19, 2020 • edited Loading

IanButterworth commented Aug 19, 2020

fonsp commented Aug 19, 2020

IanButterworth commented Aug 19, 2020

fonsp commented Aug 19, 2020

SimonDanisch commented Aug 19, 2020

fonsp commented Aug 19, 2020

johnnychen94 left a comment • edited Loading

Choose a reason for hiding this comment

IanButterworth commented Aug 20, 2020

IanButterworth commented Aug 26, 2020

johnnychen94 commented Dec 4, 2020

johnnychen94 commented Dec 4, 2020

codecov bot commented Aug 19, 2020 •

edited

Loading

IanButterworth commented Aug 19, 2020 •

edited

Loading

fonsp commented Aug 19, 2020 •

edited

Loading

johnnychen94 left a comment •

edited

Loading