fix display(d, m, x) where m expects a json string

[cnliao]

Fix queryverse/VegaLite.jl#127.
Should be reconsidered after #755

After the fix the following works out of the box in new versions of
jupyterlab, generating an interactive VegaLite visualization, without
additional packages except Ijulia.

spec = raw"""
{
  "data": {
    "values": [
      {"a": "A","b": 28}, {"a": "B","b": 55}, {"a": "C","b": 43},
      {"a": "D","b": 91}, {"a": "E","b": 81}, {"a": "F","b": 53},
      {"a": "G","b": 19}, {"a": "H","b": 87}, {"a": "I","b": 52}
    ]
  },
  "mark": "point",
  "encoding": {
    "x": {"field": "a", "type": "ordinal"},
    "y": {"field": "b", "type": "quantitative"}
  }
}
"""
display("application/vnd.vegalite.v2+json", spec)

[stevengj]

Rather than MIME($mime), you should just give the argument ::MIME{Symbol($mime)} a name, e.g. m, and use that. This way _display_dict will be dispatched statically here.

[stevengj]

I'm worried that this will conflict with other packages, since we don't "own" this type. (i.e. it is type piracy)

Maybe define _istextmime(::MIME{Symbol($mime)}) = true here, along with a fallback definition

_istextmime(m) = istextmime(m)

and then call _istextmime rather than textmime elsewhere in IJulia.

[stevengj]

This also needs to be handled in the display_dict function, I think.

[cnliao]

It's NOT working Yet!

I'm on leave so sorry if I am delaying you. @stevengj

After looking into #755 I think work should go into better support of MIME types in general. This fix will not be necessary by then. The latest commits demonstrates how I would like to achieve this.

I think it is possible to achieve:

1. inline.jl just calls display_dict and display_mimestring, without caring about json or other nuance.
2. new mime types can be easier supported: a) new code goes close together; b) third party package can support their mime type without hacking on the ijulia side.

Using traits I think one can achieve these easily and cleanly. But I am struggling to find time to contribute more aside from this still buggy draft.

Maybe I can have some time by the weekend. I am more than happy to see any of you to pick up the effort. All credit goes to @stevengj and @travigd of course.

[cnliao]

Should work now, but with some loose ends:

1. Adding a new MIME type does not really work if you just add it to ijulia_mime_types and let display_dict knows about it. You also have to specialize display* for it. It was done in inline.jl. It is now done in an updated register_mime.
2. The workarounds in inline.jl for x-latex and javascript do not fit in. It seems those workarounds tries to support mime types that ipython/jupyter does not natively understand. I think "construct content that ipython understand" and "support a user mime type" are different goals. For the latter the user is responsible for providing a mapping from his/her mime type to one that ipython understands, AFAIK such mechanism no yet exists in IJulia. Maybe ijulia_mime_types should be renamed to ipython_mime_types to make that distinction clear.

Am open to comments/opinions/etc.

[twavv]

@cnliao Apologies if if I'm off base, but, if your goal is still to just be display arbitrary data presented as a specific MIME type, couldn't we just have a

function displayraw(x::MIME, s) end

style function?

In particular, you could have displayraw(::MIME, s::AbstractString) which will transmit the data as a string and displayraw(::MIME, j::JSONText) which will transmit the data as-is. So your use case would be something like

displayraw(MIME("application/vnd.vegalite.v2+json"), JSONText(spec))

I think the implementation would just be something along the lines of sending a JSON data bundle with only that MIME key defined.

[cnliao]

@travigd After reading the Julia doc on multimedia display, people( at least myself) expect
display(MIME("application/vnd.vegalite.v2+json"), spec::String) to work out of the box. They (I ) may not want to learn about displayraw`. I don't think user should care about JSONText either.

I also think there is an issue with the test case of vnd.ijulia.friendly-binary. display(friend) works, but not display(MIME("...friendly-binary"), friend). Moreover, displayable(::InlineDisplay, MIME("...friendly-binary"), friend) returns false because istextmime returns false. (FYI that logic is in inline.jl).

[davidanthoff]

I haven't fully reviewed this, but just want to make sure the following is still true for VegaLite.jl: if someone just uses the package in the usual way in say JupyterLab, IJulia will still send both the vega-lite MIME version and a PNG back to JupyterLab, right? That is quite important, so that notebooks that are created in JupyterLab also view properly in the traditional notebook that doesn't support the vega-lite MIME type.

[cnliao]

IJulia will still send both the vega-lite MIME version and a PNG back to JupyterLab, right?

@davidanthoff True, kind of. The png is always in the resulting .ipynb file, but I don't think Ijulia generates that. I guess that ipython takes care of always generating an "image/png" entry under the "outputs" key of the cell even if you only send it a json specification (like via display("application/vnd.vegalite.v2+json", spec).

[twavv]

@cnliao That's not quite right. The notebook doesn't do any converting itself, the only thing that's included in the notebook is that the kernel (IJulia in this case) sends.

Which is why it's generally better to define a new MIME type to be included in the bundle that to just send that one specific MIME type (so the notebook can select the richest it can render).

All of which is to say the IJulia is what generates (or at least that triggers the code that generates) the PNG data that's transmitted from the kernel and ultimately embedded within the notebook

[cnliao]

@travigd
Can you try this notebook?
It works for me (on jupyterlab) , displaying a proper image with interaction.
If you run and save it, and open it with a notebook(at http://addr/tree? rather than http://addr/lab?), you see a image without interaction. reevaluating in this notebook causes the image to disappear, which is expected because it seems notebooks do not handle that mime type yet.
For the code to run you have to checkout at least the last non-conflicting commit or the latest commit of this PR.

AFAIK the call to display just call send_ipython to send no more data than a json string.

FYI I am on jupyterlab 0.35.

[stevengj]

@davidanthoff, this PR is a bit messed up because of merge conflicts, I think, making it confusing right now. But yes, the display architecture still sends both image/png and application/vnd.vegalite.vX+json if both of those are showable.

My understanding is that the intention of this PR (once it is cleaned up), is not to affect display(x) at all, but only to affect the case where you pass an explicit MIME type argument to display.

[cnliao]

@stevengj Do you think it will be better if I open another PR and provide a fix to the explicit mime variant of display? Or continue with this ? I think it’s now more of a refactor of #755.

Personally I think the solution provided here is cleaner. But I am very much like to hear your suggestions.

For the Vega lite mime type, even if you call the explicit mime variant of display, the png is still included in the resulting ipynb file. I think it is jupyterlab that takes care of that rather than Ijulia.

[twavv]

After reading the Julia doc on multimedia display, people( at least myself) expect
display(MIME("application/vnd.vegalite.v2+json"), spec::String) to work out of the box. They (I ) may not want to learn aboutdisplayraw`. I don't think user should care about JSONText either.

I think the user is going to have to differentiate JSON vs non-JSON data manually. Maybe display_json would be a better name. Otherwise we'd have no way to differentiate between the fact that

display("text/plain", "foo bar")

should send a MIME bundle of the form

{ "text/plain": "foo bar" }

Whereas you'd like to see (I think)

display("application/vnd.vegalite.v2+json", "{...}")

yielding a MIME bundle of the form

{ "application/vnd.vegalite.v2+json": { ... } }

There are some heuristics (does the MIME contain +json? is the payload wrapped in { and }?) but nothing foolproof.

[cnliao]

@travigd
For a less controversial case, I would like to see

Using VegaLite
display("application/vnd.vegalite.v2+json", vl"{...}")

yielding a MIME bundle of the form

{ "application/vnd.vegalite.v2+json": { ... } }

Because

display(vl"{...}")

already yields a MIME bundle of that form (with more MIME types).

For that to work, VegaLite.jl only has to overload Base.show for a type it owns (VLSpec). I think it is good and want that to work with display(mime"...", vl"{...}") too.

[twavv]

Ah, okay. I'm not sure that there is going to be a good way around that besides a displayjson method because IJulia can't do any kind of introspection to determine whether or not the data returned by show(::IO, ::MIME"...vegalite...", ::VLSpec) is going to JSON and whether or not it should be embedded exactly as is (à la JSONText) or as a string.

The thorn seems, to me, to be that it's hard to determine whether or not show will return JSON data or not, and whether or not it should be embeded as-is or if it should be sent as a string. So how about defining a isjsonmime to match istextmime? That would also eliminate the need for separate MIME vectors that was implemented in #755 I think.

tl;dr: how about

"""
Return true if the MIME type when `Base.show`'d returns valid JSON.

If true, the JSON is embeded within the Jupyter display_data dict rather than being
encoded as a string.
"""
isjsonmime(::MIME) = false
isjsonmime(m::AbstractString) = isjsonmime(MIME(m))
isjsonmime(::MIME"application/vnd.vegalite.v2+json") = true

and modify display(::InlineDisplay) to be more like

function jupyter_mimerepr(m::MIME, x)
    if isjsonmime(m)
        return JSONText(limitmimestring(m, x))
    end
    return limitmimestring(m, x)
end

function display(d::InlineDisplay, ::MIME{Symbol($mime)}, x)
     send_ipython(publish[],
                 msg_pub(execute_msg, "display_data",
                         Dict(
                              "metadata" => metadata(x), # optional
                              "data" => Dict($mime => jupyter_mimerepr(MIME($mime), x))
                         ))
                 )
end

(and also refactor some of #755 to avoid duplication).

@stevengj @cnliao How does all that sound?

[cnliao]

@travigd
That's almost what I do in the recent commits. I just used traits instead of if isjsonmime, and jupyter_mimerepr is named display_mimestring in my commits.

# trait cliche
abstract type MIMEStringType end
jupyter_mimerepr(m::MIME, x) = jupyter_mimerepre(mimestringtype(m), m, x)
jupyter_mimerepr(::Type{T}, m::MIME, x) where{T<:MIMEStringType} =jupyter_mimerepr(T(), m, x)
jupyter_mimerepr(::MIMEStringType, m::MIME, x) = error("unimplemented")


# default repr
struct RawMIMEString <: MIMEStringType end
mimestringtype(m::MIME) = RawMIMEString
jupyter_mimerepr(::RawMIMEString, m::MIME, x) = (m, limitstringmime(m, x))

# json repr
struct JSONMIMEString <: MIMEStringType end
mimestringtype(::MIME"application/vnd.vegalite.v2+json") = JSONMIMEString
jupyter_mimerepr(::JSONMIMEString, m::MIME, x) = (m, JSON.JSONText(limitstringmime(m, x)))

Which one is better style is still open to discussion I think.

Moreover, I think display_dict should also call these methods.

edit:
I think providing a display(::InlineDisplay, ::MIME, x) method should be mandatory so I move that definition into register_mime.