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

Remove vega v5 wrappers #2829

Merged
merged 32 commits into from
Jan 25, 2023
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
637820a
restructure, put template code together
mattijn Jan 8, 2023
92af2c6
remove vega5 schema generation wrapper code
mattijn Jan 8, 2023
3d440ba
remove vega folder from altair
mattijn Jan 8, 2023
cb72d6d
remove vega v5 magic functions
mattijn Jan 8, 2023
9c00973
remove vega as option for mimebundle
mattijn Jan 8, 2023
ecc8262
remove vega as option for save.py
mattijn Jan 8, 2023
7b06730
remove vega test from test_mimebundle
mattijn Jan 8, 2023
a628d06
remove vega tests folder
mattijn Jan 8, 2023
3e5fdf2
update NOTES_FOR_MAINTAINERS
mattijn Jan 8, 2023
052750c
update pyproject and setup files
mattijn Jan 8, 2023
f27234c
remove vega tests from test_magics
mattijn Jan 8, 2023
a288f35
remove vega docs from importing.rst
mattijn Jan 8, 2023
6b82076
remove `test_vegalite_to_vega_mimebundle` in test_mimebundle
mattijn Jan 8, 2023
c596abe
black
mattijn Jan 8, 2023
cbdb9a7
allow export format as vega in mimebundle.py
mattijn Jan 14, 2023
260e766
Revert "remove vega test from test_mimebundle"
mattijn Jan 14, 2023
bca0c56
Revert "remove `test_vegalite_to_vega_mimebundle` in test_mimebundle"
mattijn Jan 14, 2023
718ee9a
Revert "remove vega test from test_mimebundle"
mattijn Jan 14, 2023
c535a1a
use not equal to over is not
mattijn Jan 14, 2023
6bf8be9
allow vega as format but not as mode in save.py
mattijn Jan 14, 2023
dd1b1b4
change `test_spec_to_vega_mimebundle`
mattijn Jan 15, 2023
eff84c9
mention mode 'vega-lite' as preferred in warning
mattijn Jan 15, 2023
702ebba
refactor save function
mattijn Jan 15, 2023
b6fabee
add a line in release note
mattijn Jan 15, 2023
bf6b389
Merge branch 'master' into remove-vega-v5-wrappers
mattijn Jan 15, 2023
1ed3f6f
Apply suggestions from code review
mattijn Jan 24, 2023
f62ccdf
remove warnings
mattijn Jan 25, 2023
5410da5
remove warnings
mattijn Jan 25, 2023
838896e
remove warning from test
mattijn Jan 25, 2023
32d581e
solve conflicts to rebase
mattijn Jan 25, 2023
441db16
Merge branch 'master' into remove-vega-v5-wrappers
mattijn Jan 25, 2023
c009c2f
remove unused warnings package from import
mattijn Jan 25, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions NOTES_FOR_MAINTAINERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@

The core Python API for Altair can be found in the following locations:

- ``altair/vegalite/v2/schema/``
- ``altair/vegalite/v1/schema/``
- ``altair/vega/v3/schema/``
- ``altair/vega/v2/schema/``
- ``altair/vegalite/v5/schema/``

All the files within these directories are created automatically by running
the following script from the root of the repository:
Expand All @@ -19,19 +16,19 @@ $ python tools/generate_schema_wrapper.py

This script does a couple things:

- downloads the appropriate schema files from the specified vega and vega-lite
- downloads the appropriate schema files from the specified vega-lite
release versions & copies the JSON file to the appropriate ``schema``
directory
- generates basic low-level schemapi wrappers from the definitions within
the schema: this is put in the ``schema/core.py`` file
- generates a second layer of higher level wrappers for some vega-lite
functionality; this is put in ``schema/channels.py`` and ``schema/mixins.py``

The script output is designed to be deterministic; if vega/vega-lite versions
are not changed, then running the script should overwrite the schema wrappers
The script output is designed to be deterministic; if the vega-lite version
is not changed, then running the script should overwrite the schema wrappers
with identical copies.

## Updating the Vega & Vega-Lite versions
## Updating the Vega-Lite version

The vega & vega-lite versions for the Python code can be updated by manually
changing the ``SCHEMA_VERSION`` definition within
Expand All @@ -40,7 +37,7 @@ changing the ``SCHEMA_VERSION`` definition within
This will update all of the automatically-generated files in the ``schema``
directory for each version, but please note that it will *not* update other
pieces (for example, the core of the Altair API, including methods and
doc strings within ``altair/vegalite/v2/api.py``.
doc strings within ``altair/vegalite/v5/api.py``.
These additional methods have fairly good test coverage, so running the test
suite should identify any inconsistencies:
```
Expand Down
3 changes: 1 addition & 2 deletions altair/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@


def load_ipython_extension(ipython):
from ._magics import vega, vegalite
from ._magics import vegalite

ipython.register_magic_function(vega, "cell")
ipython.register_magic_function(vegalite, "cell")
74 changes: 2 additions & 72 deletions altair/_magics.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""
Magic functions for rendering vega/vega-lite specifications
Magic functions for rendering vega-lite specifications
"""
__all__ = ["vega", "vegalite"]
__all__ = ["vegalite"]

import json
import warnings
Expand All @@ -14,7 +14,6 @@
from altair.vegalite import v3 as vegalite_v3
from altair.vegalite import v4 as vegalite_v4
from altair.vegalite import v5 as vegalite_v5
from altair.vega import v5 as vega_v5

try:
import yaml
Expand All @@ -25,7 +24,6 @@


RENDERERS = {
"vega": {"5": vega_v5.Vega},
"vega-lite": {
"3": vegalite_v3.VegaLite,
"4": vegalite_v4.VegaLite,
Expand All @@ -35,10 +33,6 @@


TRANSFORMERS = {
"vega": {
# Vega doesn't yet have specific data transformers; use vegalite
"5": vegalite_v5.data_transformers,
},
"vega-lite": {
"3": vegalite_v3.data_transformers,
"4": vegalite_v4.data_transformers,
Expand Down Expand Up @@ -76,70 +70,6 @@ def _get_variable(name):
return ip.user_ns[name]


@magic_arguments.magic_arguments()
@magic_arguments.argument(
"data",
nargs="*",
help="local variable name of a pandas DataFrame to be used as the dataset",
)
@magic_arguments.argument("-v", "--version", dest="version", default="v5")
@magic_arguments.argument("-j", "--json", dest="json", action="store_true")
def vega(line, cell):
"""Cell magic for displaying Vega visualizations in CoLab.

%%vega [name1:variable1 name2:variable2 ...] [--json] [--version='v5']

Visualize the contents of the cell using Vega, optionally specifying
one or more pandas DataFrame objects to be used as the datasets.

If --json is passed, then input is parsed as json rather than yaml.
"""
args = magic_arguments.parse_argstring(vega, line)

existing_versions = {"v5": "5"}
version = existing_versions[args.version]
assert version in RENDERERS["vega"]
Vega = RENDERERS["vega"][version]
data_transformers = TRANSFORMERS["vega"][version]

def namevar(s):
s = s.split(":")
if len(s) == 1:
return s[0], s[0]
elif len(s) == 2:
return s[0], s[1]
else:
raise ValueError("invalid identifier: '{}'".format(s))

try:
data = list(map(namevar, args.data))
except ValueError:
raise ValueError("Could not parse arguments: '{}'".format(line))

if args.json:
spec = json.loads(cell)
elif not YAML_AVAILABLE:
try:
spec = json.loads(cell)
except json.JSONDecodeError:
raise ValueError(
"%%vega: spec is not valid JSON. "
"Install pyyaml to parse spec as yaml"
)
else:
spec = yaml.load(cell, Loader=yaml.SafeLoader)

if data:
spec["data"] = []
for name, val in data:
val = _get_variable(val)
prepped = _prepare_data(val, data_transformers)
prepped["name"] = name
spec["data"].append(prepped)

return Vega(spec)


@magic_arguments.magic_arguments()
@magic_arguments.argument(
"data",
Expand Down
34 changes: 16 additions & 18 deletions altair/utils/mimebundle.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from .html import spec_to_html
import warnings
from .deprecation import AltairDeprecationWarning
from ..vegalite.v5.data import data_transformers


Expand All @@ -12,18 +14,18 @@ def spec_to_mimebundle(
engine=None,
**kwargs,
):
"""Convert a vega/vega-lite specification to a mimebundle
"""Convert a vega-lite specification to a mimebundle

The mimebundle type is controlled by the ``format`` argument, which can be
one of the following ['html', 'json', 'png', 'svg', 'pdf', 'vega', 'vega-lite']
one of the following ['html', 'json', 'png', 'svg', 'pdf', 'vega', 'vega-lite']
mattijn marked this conversation as resolved.
Show resolved Hide resolved

Parameters
----------
spec : dict
a dictionary representing a vega-lite plot spec
format : string {'html', 'json', 'png', 'svg', 'pdf', 'vega', 'vega-lite'}
format : string {'html', 'json', 'png', 'svg', 'pdf', 'vega', 'vega-lite'}
mattijn marked this conversation as resolved.
Show resolved Hide resolved
the file format to be saved.
mode : string {'vega', 'vega-lite'}
mode : string {'vega-lite'}
The rendering mode.
vega_version : string
The version of vega.js to use
Expand All @@ -43,16 +45,16 @@ def spec_to_mimebundle(

Note
----
The png, svg, pdf, and vega outputs require the altair_saver package
The png, svg, vega and pdf outputs require the vl-convert or altair_saver package
to be installed.
"""
if mode not in ["vega", "vega-lite"]:
raise ValueError("mode must be either 'vega' or 'vega-lite'")
if mode != "vega-lite":
if mode == "vega":
warnings.warn(
"mode 'vega' is deprecated, use 'vega-lite'", AltairDeprecationWarning
mattijn marked this conversation as resolved.
Show resolved Hide resolved
)
raise ValueError("mode must be 'vega-lite'")

if mode == "vega" and format == "vega":
if vega_version is None:
raise ValueError("Must specify vega_version")
return {"application/vnd.vega.v{}+json".format(vega_version[0]): spec}
if format in ["png", "svg", "pdf", "vega"]:
return _spec_to_mimebundle_with_engine(
spec, format, mode, engine=engine, **kwargs
Expand All @@ -68,17 +70,13 @@ def spec_to_mimebundle(
)
return {"text/html": html}
if format == "vega-lite":
assert mode == "vega-lite" # sanity check: should never be False
if mode == "vega":
raise ValueError("Cannot convert a vega spec to vegalite")
if vegalite_version is None:
raise ValueError("Must specify vegalite_version")
return {"application/vnd.vegalite.v{}+json".format(vegalite_version[0]): spec}
if format == "json":
return {"application/json": spec}
raise ValueError(
"format must be one of "
"['html', 'json', 'png', 'svg', 'pdf', 'vega', 'vega-lite']"
"format must be one of ['html', 'json', 'png', 'svg', 'pdf', 'vega', 'vega-lite']"
)


Expand All @@ -91,7 +89,7 @@ def _spec_to_mimebundle_with_engine(spec, format, mode, **kwargs):
a dictionary representing a vega-lite plot spec
format : string {'png', 'svg', 'pdf', 'vega'}
the format of the mimebundle to be returned
mode : string {'vega', 'vega-lite'}
mode : string {'vega-lite'}
The rendering mode.
engine: string {'vl-convert', 'altair_saver'}
the conversion engine to use
Expand Down Expand Up @@ -147,7 +145,7 @@ def _validate_normalize_engine(engine, format):

engine : {None, 'vl-convert', 'altair_saver'}
the user-provided engine string
format : string {'png', 'svg', 'pdf', 'vega'}
format : string {'png', 'svg', 'pdf'}
mattijn marked this conversation as resolved.
Show resolved Hide resolved
the format of the mimebundle to be returned
"""
# Try to import vl_convert
Expand Down
74 changes: 47 additions & 27 deletions altair/utils/save.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json
import pathlib
import warnings
from .deprecation import AltairDeprecationWarning

from .mimebundle import spec_to_mimebundle

Expand All @@ -15,6 +16,48 @@ def write_file_or_filename(fp, content, mode="w"):
fp.write(content)


def set_inspect_format_argument(format, fp, inline):
"""Inspect the format argument in the save function"""
if format is None:
if isinstance(fp, str):
format = fp.split(".")[-1]
elif isinstance(fp, pathlib.PurePath):
format = fp.suffix.lstrip(".")
mattijn marked this conversation as resolved.
Show resolved Hide resolved
else:
raise ValueError(
"must specify file format: "
"['png', 'svg', 'pdf', 'html', 'json', 'vega']"
mattijn marked this conversation as resolved.
Show resolved Hide resolved
)

if format != "html" and inline:
warnings.warn("inline argument ignored for non HTML formats.")

return format


def set_inspect_mode_argument(mode, embed_options, spec, vegalite_version):
"""Inspect the mode argument in the save function"""
if mode is None:
if "mode" in embed_options:
mode = embed_options["mode"]
elif "$schema" in spec:
mode = spec["$schema"].split("/")[-2]
else:
mode = "vega-lite"

if mode != "vega-lite":
if mode == "vega":
warnings.warn(
Copy link
Contributor

Choose a reason for hiding this comment

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

Same remark here about the deprecation warning

"mode 'vega' is deprecated, use 'vega-lite'", AltairDeprecationWarning
)
raise ValueError("mode must be 'vega-lite', " "not '{}'".format(mode))

if mode == "vega-lite" and vegalite_version is None:
raise ValueError("must specify vega-lite version")

return mode


def save(
chart,
fp,
Expand Down Expand Up @@ -45,7 +88,7 @@ def save(
the format to write: one of ['json', 'html', 'png', 'svg', 'pdf'].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
the format to write: one of ['json', 'html', 'png', 'svg', 'pdf'].
the format to write: one of ['json', 'html', 'png', 'svg', 'pdf', 'vega', 'vega-lite'].

Only add the vega-lite suggestion if it is also supported in the rest of save

If not specified, the format will be determined from the filename.
mode : string (optional)
Either 'vega' or 'vegalite'. If not specified, then infer the mode from
Must be 'vegalite'. If not specified, then infer the mode from
the '$schema' property of the spec, or the ``opt`` dictionary.
If it's not specified in either of those places, then use 'vegalite'.
mattijn marked this conversation as resolved.
Show resolved Hide resolved
vega_version : string (optional)
Expand Down Expand Up @@ -81,34 +124,11 @@ def save(
if embed_options is None:
embed_options = {}

if format is None:
if isinstance(fp, str):
format = fp.split(".")[-1]
elif isinstance(fp, pathlib.PurePath):
format = fp.suffix.lstrip(".")
else:
raise ValueError(
"must specify file format: " "['png', 'svg', 'pdf', 'html', 'json']"
)
format = set_inspect_format_argument(format, fp, inline)

spec = chart.to_dict()

if mode is None:
if "mode" in embed_options:
mode = embed_options["mode"]
elif "$schema" in spec:
mode = spec["$schema"].split("/")[-2]
else:
mode = "vega-lite"

if mode not in ["vega", "vega-lite"]:
raise ValueError("mode must be 'vega' or 'vega-lite', " "not '{}'".format(mode))

if mode == "vega-lite" and vegalite_version is None:
raise ValueError("must specify vega-lite version")

if format != "html" and inline:
warnings.warn("inline argument ignored for non HTML formats.")
mode = set_inspect_mode_argument(mode, embed_options, spec, vegalite_version)

if format == "json":
json_spec = json.dumps(spec, **json_kwds)
Expand All @@ -128,7 +148,7 @@ def save(
**kwargs,
)
write_file_or_filename(fp, mimebundle["text/html"], mode="w")
elif format in ["png", "svg", "pdf"]:
elif format in ["png", "svg", "pdf", "vega"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't comment on the correct line but vega does not work here. It will try to access the mimebundle with key image/svg+xml on line 169.

Could also add support for "vega-lite" here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vega-lite cannot be supported as format. That would be json. format functions a bit like export to, or save as in other programs. If it is defined as vega it should compile the vega-lite specification into vega json.
Apparently that never has functioned.

Copy link
Contributor

Choose a reason for hiding this comment

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

Of course, you're right, that would be json!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I just noticed the following variations in the code:
https://github.com/altair-viz/altair/blob/1ed3f6f2fd3647bcf1f2e5e81dd328a0f9cf623d/altair/utils/mimebundle.py#L72-L77

It seems there is different type of json, namely, application/json and application/vnd.vegalite.v5+json (for altair 5). So I'll include vega-lite also as format, it probably would be better if it was renamed to something as vl-json.

mimebundle = spec_to_mimebundle(
spec=spec,
format=format,
Expand Down
2 changes: 0 additions & 2 deletions altair/vega/__init__.py

This file was deleted.

Loading