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

@plottest #62

Closed
oxinabox opened this issue Jul 5, 2020 · 13 comments · Fixed by #77
Closed

@plottest #62

oxinabox opened this issue Jul 5, 2020 · 13 comments · Fixed by #77

Comments

@oxinabox
Copy link
Member

oxinabox commented Jul 5, 2020

I wonder if we want something like VisualRegressionTest's @plottest that takes as input something that interacts with display. (I think? Or maybe it just calls savefig but interacting as a display seems more robust)

Right now it's annoying to use this with plots since they don't return object that can be compared.

Then I could stop using VisualRegressionTests

@johnnychen94
Copy link
Member

johnnychen94 commented Jul 6, 2020

We should provide a guaranteed way to support extensions, either via creating another *ReferenceTest package or via Requires.

And we can then move image-related functionality out from the core.

@johnnychen94
Copy link
Member

johnnychen94 commented Jul 6, 2020

I would prefer to compare data instead of figures as figures can be unstable due to backend changes.

@oxinabox
Copy link
Member Author

oxinabox commented Jul 6, 2020

but sometime you want to test against that.
E.g. i have tests that are there and make sure my code works desite bugs in GR right now.

@juliohm
Copy link
Contributor

juliohm commented Feb 3, 2021

I've just refactored the test suite of ImageInpainting.jl and replaced VisualRegressionTests.jl to ReferenceTests.jl and it is so much cleaner now. Thanks for this package!

Next, I will try to refactor all packages in the GeoStats.jl stack to use ReferenceTests.jl as well, and they are full of @plottest calls (I contributed this macro to the package). I am wondering how I should proceed.

Suppose I have a plot object with the GR backend p = plot(foo). What is the cleanest method to compare p using @test_reference?

@oxinabox
Copy link
Member Author

oxinabox commented Feb 3, 2021

I think show(iobuffer, "image/png", plot) should work?
(Might need to pull the mimetype out of the reference file?)

@juliohm
Copy link
Contributor

juliohm commented Feb 4, 2021

Nice, I don't know if I understood the mimetype comment, but I will give it a try, hopefully soon.

@juliohm
Copy link
Contributor

juliohm commented Feb 9, 2021

@oxinabox what should the iobuffer be exactly? I tried to do iobuffer = IOBuffer() but don't know if that makes sense.

@oxinabox
Copy link
Member Author

oxinabox commented Feb 9, 2021

I am not sure, I don't remember how that bit of the code works.
But at the end of the day you need to get the plot into some kind of in-memory stucture that can be compared to the on-disk PNG, no?

@juliohm
Copy link
Contributor

juliohm commented Feb 9, 2021

Any example that we could copy/paste? I am searching the web for a simple example of how to convert a plot into an image without success.

@juliohm
Copy link
Contributor

juliohm commented Feb 9, 2021

I could do the following:

using Plots

io = IOBuffer()

p = scatter(rand(100))

show(io, "image/png", p) # stores data in in-memory buffer

data = take!(io) # retrieve data as a Vector{UInt8}

I have no idea how to convert this vector of int into an image.

@Evizero
Copy link
Member

Evizero commented Feb 9, 2021

Hey! I am really rusty and outdated -- and there is a possibility that my memory is somehow skewed and simply wrong -- but maybe I can provide some useful breadcrumbs.

First of all I think you are on the right track that you'd want to convert a figure into an actual image (aka colorant array) and then hook into our image comparing logic.

Now concerning the IOBuffer subtopic. In essence, what you get after take! is an array of bytes and those bytes are basically just the verbatim contents of a .png file of your scatter plot. So those bytes are encoded and not in any way some reshaped form of a colorant array.

In order to get a colorant array you have to essentially pretend its a .png file with the byte contents of data. Naturally, you'd like to avoid writing a temporary file just to read it again. Back in the day I avoided the temporary file problem by using some internal function of ImageMagick.jl (see https://github.com/Evizero/ISICArchive.jl/blob/master/src/image.jl#L122 where i parse such an encoded string with ImageMagick.readblob. Note that I dont know if that code even still works). Hopefully there is more backend-agnostic function somewhere nowadays, but this I do not know.

@timholy What is the current best practice to deserialize the contents of an image in memory (like ImageMagick.readblob used to do)

@juliohm
Copy link
Contributor

juliohm commented Feb 9, 2021

People helped on Zulip, this is the final solution with PNGFiles:

using PNGFiles

function asimage(plt)
  io = IOBuffer()
  show(io, "image/png", plt)
  seekstart(io)
  PNGFiles.load(io)
end

img = asimage(scatter(rand(100)))

@oxinabox would you like me to submit a PR with this helper macro? Perhaps @test_reference_plot "data/plot.png" plt?

@oxinabox
Copy link
Member Author

oxinabox commented Feb 9, 2021

That would be awesome.
I would get ride of so many annoying to update VisualRegressionTests uses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants