Generic show-based log_image?

[tkf]

It looks like how Plots.jl and PyPlot.jl are handled is very generic. So, why not accept any object that is showable as image/png? What I mean is something like this:

function log_image(lg::TBLogger, name::AbstractString, obj; step=nothing)
    if !showable("image/png", obj)
        throw(ArgumentError("Cannot be logged as an image: $obj"))
        # or maybe `@error`?
    end
    pb = PipeBuffer()
    show(pb, "image/png", obj)
    img = PNG(pb)
    log_keyval(lg, name, img, step)
end

It would be very useful when creating plots in other libraries like VegaLite.jl which supports show(io, "image/png", plot).

[PhilipVinc]

Response:

• Indeed. I had thought about enabling this while adding the Plots functionality and then forgot.

Additional reflections:
The explicit interface (log_xxx) is more like a second class citizen, as the logger interface is (often) much more convenient and flexible, because you can extend it easily.

The logger interface (the one based on @logmsg) could already handle anything you throw at it provided you define the method preprocess(name, plot::VegaPlot, data) = ....

The key point here is that we dispatch based on types, and we only handle properly types we know about (+ text-based fallback). So to handle all plot types, we should define the preprocess method for all Plot types we know about (using Requires.jl), but that isn't viable (I think).
Moreover, the VegaPlot type (or any showable type actually) are structs, and we would dismantle them without realising that they can actually be rendered.

@oxinabox do you think that adding a check on preprocess(name, val::Any, data) to check if it's showable to PNG would be a good solution?

I'm not convinced, as if the user defined some show method for his custom types that otherwise he would like to dismantle into it's components, we could risk rendering to png types we are not supposed to...

[oxinabox]

I've never been fully happy with the way preprocess works in the first place.

I think we should add the log_image feature discussed.
and then open another issue and think more about preprocess.
(E.g. I want a generic fallback to checking showable("text/html",...) (and for a number of other rplain text mimetypes) to use the tensorboard text/markdown screen. But working out how to have both that, and destructuring requires thought)

[tkf]

Why not add an explicit interface like this?

@info "prefix" myplot=TBImage(myplot)

It doesn't work well with current TBImage but I'd imagine it's easy to support by

abstract type TBImage <: WrapperLogType end

struct RawTBImage <: TBImage  # renamed from current TBImage
    data::AbstractArray
    format::ImageFormat
end

struct GenericTBImage <: TBImage
    object
end

Then you can define (say)

function Base.convert(T::Type{PNG}, image::GenericTBImage)
    pb = PipeBuffer()
    show(pb, "image/png", image.object)
    return PNG(pb)
end

I guess it'd be nice if there is a generic wrapper type for PNG like HTML and Text in Base so that you can do

@info "prefix" myplot=PNG(myplot)

There is https://github.com/tkf/DisplayAs.jl for this kind of purpose but it's not super popular.

[PhilipVinc]

We don't even need to create a new type. Just adding a new special value to ImageFormat (say, 0), and a new single-argument constructor to TBImage and this should work.

I'll add this to #50.

But this is a stopgap, and I agree with @oxinabox in the fact that we should find some way to better handle those types by default, without special user input

[tkf]

I think it's better to have low-level explicit API if you provide high-level slightly magic API. But if you auto-detect MIMEs then I can use DisplayAs to enforce the MIME to be used. So I'm personally OK with not having the explicit API in this case.

[PhilipVinc]

The new generic case of TBImage(yourplot) is the low level, tell-you-yourself, API.
The magic would be detecting that yourplot should be logged as an image, even without the explicit override TBImage.

[tkf]

Thanks, that sounds great.