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

Vega support #153

Merged
merged 10 commits into from Mar 9, 2019
Merged

Vega support #153

merged 10 commits into from Mar 9, 2019

Conversation

mcmcgrath13
Copy link
Collaborator

This PR proposes a few changes to increase support for Vega specifications:

  • Update the MIME type for VGSpecs to "application/vnd.vega.v4+json", which is the current standard for JupyterLab (have to install an extension for the current v3 to show interactively)
  • Create an AbstractVegaSpec type to simplify some mostly duplicate functions
  • Add a section to the docs on using Vega and how to use it in a function (I had some trouble getting this to work and it feels slightly hack-y - @fredo-dedup do you have a better way to do this?)

I noticed the examples folder seems to use an outdated syntax - can that be removed as well?

This PR shouldn't be merged until the corresponding IJulia PR is merged so they are ready to support the v4 mime type.

I expect this will need to be updated shortly as Vega recently released v5.

@codecov-io
Copy link

codecov-io commented Mar 8, 2019

Codecov Report

Merging #153 into master will decrease coverage by 1.05%.
The diff coverage is 41.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #153      +/-   ##
==========================================
- Coverage   69.28%   68.23%   -1.06%     
==========================================
  Files          17       17              
  Lines         661      617      -44     
==========================================
- Hits          458      421      -37     
+ Misses        203      196       -7
Impacted Files Coverage Δ
src/VegaLite.jl 100% <ø> (ø) ⬆️
src/vgspec.jl 100% <ø> (ø) ⬆️
src/rendering/io.jl 100% <ø> (+3.22%) ⬆️
src/rendering/fileio.jl 25% <0%> (ø) ⬆️
src/vlspec.jl 100% <100%> (ø) ⬆️
src/rendering/show.jl 86.27% <40%> (+0.71%) ⬆️

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 dd93463...746e8f6. Read the comment docs.

@coveralls
Copy link

coveralls commented Mar 8, 2019

Coverage Status

Coverage increased (+0.2%) to 68.947% when pulling 746e8f6 on mcmcgrath13:vega_support into dd93463 on fredo-dedup:master.

@mcmcgrath13
Copy link
Collaborator Author

Travis is failing on mac due to an issue with the Cairo build - I need to dig into this

Copy link
Member

@davidanthoff davidanthoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, this is fantastic and in really good shape!! Only one comment, and then we can merge this :)

Project.toml Outdated Show resolved Hide resolved
@davidanthoff
Copy link
Member

Travis is failing on mac due to an issue with the Cairo build - I need to dig into this

That unfortunately happens regularly... It is really frustrating, the whole BinDeps.jl binary provider story that Cairo.jl utilizes is just not reliable and seems to break a lot... I think the only proper solution is that someone creates a BinaryBuilder.jl style solution for Cairo.jl and Rsvg.jl. But that implies Gtk.jl also needs to be ported, and at that point it is a mega project...

In any case, that failure is clearly not related to your PR, so I think we can merge things here independently of the Cairo.jl situation.

@mcmcgrath13
Copy link
Collaborator Author

@davidanthoff that's good to know about Cairo... (kind of - it's always nicer when things just work). I've gitignored the project.toml and when this is merged I'll do another PR with just the Project.toml and the updated travis script so it can be merged whenever attobot is ready.

On the IJulia side, I've updated that PR to preemptively include the mime types for vega v5 and vegalite v3 so we should be able to switch those over whenever they're supported by jupyter.

@davidanthoff davidanthoff merged commit 1758152 into queryverse:master Mar 9, 2019
@davidanthoff
Copy link
Member

Super, thanks a lot!

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

Successfully merging this pull request may close these issues.

None yet

4 participants