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

refactoring: remove vega and vegalite wrappers #2817

Closed
mattijn opened this issue Jan 8, 2023 · 8 comments
Closed

refactoring: remove vega and vegalite wrappers #2817

mattijn opened this issue Jan 8, 2023 · 8 comments

Comments

@mattijn
Copy link
Contributor

mattijn commented Jan 8, 2023

Continuation of the discussion that started from this comment and onwards.

Vega 5 specifications can currently be rendered using Altair, with support of interactivity:

import altair.vega.v5 as alt
import json
vg_spec = r"""
{
  "$schema": "https://vega.github.io/schema/vega/v5.json",
  "description": "A basic bar chart example, with value labels shown upon mouse hover.",
  "width": 400,
  "height": 200,
  "padding": 5,
  "data": [
    {
      "name": "table",
      "values": [
        {"category": "A", "amount": 28},
        {"category": "B", "amount": 55},
        {"category": "C", "amount": 43},
        {"category": "D", "amount": 91},
        {"category": "E", "amount": 81},
        {"category": "F", "amount": 53},
        {"category": "G", "amount": 19},
        {"category": "H", "amount": 87}
      ]
    }
  ],
  "scales": [
    {
      "name": "xscale",
      "type": "band",
      "domain": {"data": "table", "field": "category"},
      "range": "width",
      "padding": 0.05,
      "round": true
    },
    {
      "name": "yscale",
      "domain": {"data": "table", "field": "amount"},
      "nice": true,
      "range": "height"
    }
  ],
  "axes": [
    {"orient": "bottom", "scale": "xscale"},
    {"orient": "left", "scale": "yscale"}
  ],
  "marks": [
    {
      "type": "rect",
      "from": {"data": "table"},
      "encode": {
        "enter": {
          "x": {"scale": "xscale", "field": "category"},
          "width": {"scale": "xscale", "band": 1},
          "y": {"scale": "yscale", "field": "amount"},
          "y2": {"scale": "yscale", "value": 0}
        },
        "update": {"fill": {"value": "steelblue"}},
        "hover": {"fill": {"value": "red"}}
      }
    }
  ]
}
"""
alt.vega(json.loads(vg_spec))

image

Now, one can also use vl-convert (pip/conda install vl-convert-python) to render these as png or svg in a notebook:

import vl_convert as vlc
from IPython.display import Image
from IPython.display import SVG, display

#vg_png = vlc.vega_to_png(vg_spec)  # Image(vg_png)
vg_svg = vlc.vega_to_svg(vg_spec)
display(SVG(vg_svg))

image
But as far as I can tell it does not support the interactivity (btw just like a svg-export from the Vega Editor with above example).

Introducing errors in the spec, propagate JavaScript warnings back to the notebook, in both cases:
Changing eg: "from": {"data": "table"}, to "from": {"data": "FOO"},
Results with Altair variant:

Javascript Error: Undefined data set name: "FOO"

And with vl-convert variant:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[19], line 6
      3 from IPython.display import SVG, display
      5 #vg_png = vlc.vega_to_png(vg_spec)  # Image(vg_png)
----> 6 vg_svg = vlc.vega_to_svg(vg_spec)
      7 display(SVG(vg_svg))

ValueError: Vega to SVG conversion failed:
Error: Undefined data set name: "FOO"
    at b (https://cdn.skypack.dev/-/vega-util@v1.17.0-uRskU0IBL2vWCP4Va8OC/dist=es2020,mode=imports,min/optimized/vega-util.js:1:324)
    at ue.getData (https://cdn.skypack.dev/-/vega-parser@v6.1.4-PbzshukasZitnbnp29GQ/dist=es2020,mode=imports,min/optimized/vega-parser.js:1:45069)
    at At (https://cdn.skypack.dev/-/vega-parser@v6.1.4-PbzshukasZitnbnp29GQ/dist=es2020,mode=imports,min/optimized/vega-parser.js:1:21597)
    at Sa (https://cdn.skypack.dev/-/vega-parser@v6.1.4-PbzshukasZitnbnp29GQ/dist=es2020,mode=imports,min/optimized/vega-parser.js:1:21499)
    at Bt (https://cdn.skypack.dev/-/vega-parser@v6.1.4-PbzshukasZitnbnp29GQ/dist=es2020,mode=imports,min/optimized/vega-parser.js:1:24270)
    at https://cdn.skypack.dev/-/vega-parser@v6.1.4-PbzshukasZitnbnp29GQ/dist=es2020,mode=imports,min/optimized/vega-parser.js:1:39099
    at Array.forEach (<anonymous>)
    at hn (https://cdn.skypack.dev/-/vega-parser@v6.1.4-PbzshukasZitnbnp29GQ/dist=es2020,mode=imports,min/optimized/vega-parser.js:1:39088)
    at al (https://cdn.skypack.dev/-/vega-parser@v6.1.4-PbzshukasZitnbnp29GQ/dist=es2020,mode=imports,min/optimized/vega-parser.js:1:39762)
    at Module.sl (https://cdn.skypack.dev/-/vega-parser@v6.1.4-PbzshukasZitnbnp29GQ/dist=es2020,mode=imports,min/optimized/vega-parser.js:1:47716)

Since it is not the core of Altair, we can decide for maintenance purposes to remove the Vega (currently v5) wrappers and leave this to vl-convert if people like to make use of this within a notebook environment.

Altair had until this commit support in the main version on Github support for the following Vega-Lite versions:

Altair Version Vega-Lite Version Vega Version
4.3.0.dev0 v3.4.0, v4.17.0, v5.2.0 v5.21.0

Three versions of Vega-Lite seems too many, and we are still discussing if we can reduce it to only a single version of Vega-Lite. But the v3 version is obsolete anyway. These packages/tests can safely be removed from main.


comment by @binste:

For completeness: JupyterLab has builtin support for Vega files which can be rendered with interactivity:

image

JupyterLab docs on this

You can also plot Vega specs in a notebook without converting to svg or png first so you still have the interactivity:

import json
from IPython.display import display

vg_spec = r"""
{
  "$schema": "https://vega.github.io/schema/vega/v5.json",
  "description": "A basic bar chart example, with value labels shown upon mouse hover.",
  "width": 400,
  "height": 200,
  "padding": 5,
  "data": [
    {
      "name": "table",
      "values": [
        {"category": "A", "amount": 28},
        {"category": "B", "amount": 55},
        {"category": "C", "amount": 43},
        {"category": "D", "amount": 91},
        {"category": "E", "amount": 81},
        {"category": "F", "amount": 53},
        {"category": "G", "amount": 19},
        {"category": "H", "amount": 87}
      ]
    }
  ],
  "scales": [
    {
      "name": "xscale",
      "type": "band",
      "domain": {"data": "table", "field": "category"},
      "range": "width",
      "padding": 0.05,
      "round": true
    },
    {
      "name": "yscale",
      "domain": {"data": "table", "field": "amount"},
      "nice": true,
      "range": "height"
    }
  ],
  "axes": [
    {"orient": "bottom", "scale": "xscale"},
    {"orient": "left", "scale": "yscale"}
  ],
  "marks": [
    {
      "type": "rect",
      "from": {"data": "table"},
      "encode": {
        "enter": {
          "x": {"scale": "xscale", "field": "category"},
          "width": {"scale": "xscale", "band": 1},
          "y": {"scale": "yscale", "field": "amount"},
          "y2": {"scale": "yscale", "value": 0}
        },
        "update": {"fill": {"value": "steelblue"}},
        "hover": {"fill": {"value": "red"}}
      }
    }
  ]
}
"""

display({
    "application/vnd.vega.v5+json": json.loads(vg_spec)
}, raw=True)
image

The above code with display also works in VS Code and presumably a few other notebook frontends.

@mattijn
Copy link
Contributor Author

mattijn commented Jan 8, 2023

Thanks for the comment @binste, given this alternative it is fine by me to refactor the Vega v5 wrappers out of Altair.

Regarding the Vega-Lite wrappers, another option would be to prepare a 4.3 version of Altair with some of the new features excluding the changes required for vega-lite v5 and from there refactor Altair v5 with just a single Vega-Lite v5 wrapper?

@joelostblom
Copy link
Contributor

I am +1 on removing the Vega 5 code and it seems like Jake also had this in mind based on the comment that @binste linked.

Regarding a 4.3 version, I think there are some nice things with this, but it would involve a lot of rearranging or cherrypicking of commits to create the 4.3 release. I would rather spend that time on working toward a 5.0 release, and recommend people to update.

Based on the discussion in #2785 I remain in favor of only supporting Vega-Lite 5 in Altair 5. I agree with the idea that pinning of the Vega-Lite version is best done via pinning the Altair version, rather than using the newest version of Altair with specific Vega-Lite versions. It does also seem that this could add significant maintenance cost in terms of figuring out which new Altair features are safe to backport to the older Vega-Lite version and which should only be applied to the latest Vega-Lite version. Given that the upgrade from Altair 4 for 5 has been quite the undertaking, I'm can't help to lean towards the solution that is easier to maintain and upgrade in the future.

@mattijn
Copy link
Contributor Author

mattijn commented Jan 8, 2023

Fine. At least its good documented why we are doing this. Then we agree to refactor vega v5, vega-lite v3 and vega-lite v4 out of Altair.

@joelostblom
Copy link
Contributor

@jakevdp Just a ping here in case you are able to share your thoughts on this issue since it is a bigger decision. But I understand if you are not able to and then we can go ahead with the current plan for the time being.

@ChristopherDavisUCI
Copy link
Contributor

Is Vega-Lite 5.2 what we're aiming for in the next release? On the Altair side, I don't think there will be any issue using a later version of Vega-Lite.

I'm asking here just as a possible reminder that this decision exists!

@mattijn
Copy link
Contributor Author

mattijn commented Jan 14, 2023

Altair itself will be fine, but you won’t be able to pass the tests as defined in GitHub Action, since altair_viewer is still VL v5.2. If I recall correctly.

@joelostblom
Copy link
Contributor

I had a brief look on the release notes for 5.3, 5.4, 5.5, and 5.6 and I don't see anything super important, so I am ok sticking with 5.2 and ironing out minor things in the meanwhile. But maybe open an issue to track it @ChristopherDavisUCI, in case it is something relatively straightforward we can do later, just before the release (candidate).

@ChristopherDavisUCI
Copy link
Contributor

Thanks! @joelostblom I opened a brief issue here: #2833

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

No branches or pull requests

3 participants