queryverse / ElectronDisplay.jl

An Electron.jl based figure and table display.
Other
85 stars 17 forks source link

Bundled vega-lite.min.js is old #26

Closed tkf closed 5 years ago

tkf commented 5 years ago

It seems that ElectronDisplay.jl bundles Vega Lite 2.6 while VegaLite.jl uses 3.0.

$ cd ~/.julia/dev/ElectronDisplay/assets/vega
$ grep -Eo '[0-9]+\.[0-9]+\.[0-9]+' vega-lite.min.js
2.6.0

$ cd ~/.julia/dev/VegaLite/deps/lib/
$ grep -Eo '[0-9]+\.[0-9]+\.[0-9]+' vega-lite.min.js
3.0.0

Could you update vega-lite.min.js?

More generally, can JavaScript libraries used by ElectronDisplay.jl and VegaLite.jl be synchronized? A rather hacky way to do it is to try to load JavaScript files from VegaLite.jl if it's in Manifest.toml. This can be done by:

pkg = Base.PkgId(Base.UUID(0x112f6efa9a025b7d90c0432ed331239a), "VegaLite")
joinpath(dirname(dirname(Base.locate_package(pkg))), "deps", "lib", "vega-lite.min.js")

and it does not require you to explicitly add VegaLite.jl to deps.

Maybe a better way to do it is to create (say) VegaJS.jl package and make it a dependency of both ElectronDisplay.jl and VegaLite.jl.

davidanthoff commented 5 years ago

VegaLite.jl tagged is still on vega-lite 2.6, only master is on 3.0.

I think the proper way to handle this is to ship multiple vega-lite versions simultaneously here in ElectronDisplay.jl, i.e. one for each major version of vega-lite, and then pick which one to use depending on the MIME type.

tkf commented 5 years ago

Thanks a lot for the super quick fix!

davidanthoff commented 5 years ago

Sure! If you could give the current version on master a try and report back whether it addresses your original issue it would be great! And then I'll tag a new release.

tkf commented 5 years ago

Hmm... So maybe I was wrong to assume the original problem I had was due to vega-lite's version? Here is a MWE:

using VegaLite
using DataFrames

df = DataFrame(
    a = 1:3,
    b = 1:3,
    c = 1:3,
)

plt = df |> @vlplot(
    repeat = [:a, :b, :c],
    columns = 2,
) +
@vlplot(
    :line,
    x = {field = :a, typ=:quantitative},
    y = {field={repeat=:repeat}, typ=:quantitative},
)

using ElectronDisplay
electrondisplay(plt)

This script shows:

image

while normal display (web browser) and electrondisplay("image/png", plt) show the correct plot:

image

When I open the devtool using Electron.toggle_devtools, there is a warning in the console:

WARN Unknown repeated value "repeat".
s @ vega-lite.min.js:1

which led me to think it was the version issue.

tkf commented 5 years ago

Actually, it looks like VegaLite.jl only supports application/vnd.vegalite.v2+json? https://github.com/fredo-dedup/VegaLite.jl/blob/e41ff3f622d2490481444704a292834c063b9a74/src/rendering/show.jl#L35-L37

I guess showable as used in #27 requires VegaLite.jl to define show for all versions?

davidanthoff commented 5 years ago

Ah, yes! Maybe VegaLite.jl master should only support application/vnd.vegalite.v3+json now? So essentially the VegaLite.jl package is always tied to one version, but this package here can display whatever version one throws at it?

But we would probably have to check whether jupyterlab e.g. supports the v3 MIME type already, otherwise we screw that scenario up...

tkf commented 5 years ago

It looks like jupyterlab bundles Vega 5 and Vega-Lite 3:

https://github.com/jupyterlab/jupyterlab/tree/31407ecb55890077f66e0e9dd2c57957524f234b/packages/vega5-extension

davidanthoff commented 5 years ago

I think this https://github.com/fredo-dedup/VegaLite.jl/pull/164 would do it. BUT, I first need to fix that branch: the schema changes in one of these updates broke the VegaLite.jl implementation... And then I also need to make sure the VS Code plot gallery supports the new MIME types.

tkf commented 5 years ago

Great. Thanks a lot!