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 #2822

Closed
wants to merge 14 commits into from
Closed

Conversation

mattijn
Copy link
Contributor

@mattijn mattijn commented Jan 8, 2023

This PR removes the Vega V5 wrappers from the Altair repository.

It removes them from the generate_schema_wrapper, magics, mimebundle, tests and docs.

Not everything related to vega is removed since the saving utilities requires vega bundles to be included.

@mattijn mattijn marked this pull request as ready for review January 8, 2023 21:41
@mattijn mattijn requested a review from joelostblom January 8, 2023 21:41
@mattijn
Copy link
Contributor Author

mattijn commented Jan 8, 2023

Can you check @joelostblom? I think I removed everything related to the Vega wrapper, but I might have missed some parts in the docs/docstring.

Copy link
Contributor

@joelostblom joelostblom left a comment

Choose a reason for hiding this comment

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

Nice, thanks for doing this. Apart from one of the deleted tests I think this looks good. I grepped the repo for "vega" and didn't notice any other places needing an update.

I noticed the name of the branch and just wanted to say that I think it makes sense to have a separate PR for removing the old Vega-Lite versions and limiting this PR to Vega as you have done now 👍

tests/utils/tests/test_mimebundle.py Show resolved Hide resolved
Copy link
Contributor

@joelostblom joelostblom left a comment

Choose a reason for hiding this comment

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

I misread the test I thought was accidentally deleted, this looks good.

@joelostblom
Copy link
Contributor

One thing I was thinking here is that maybe we should add an explicit note in the docs that Altair specs can programmatically be turned into VegaLite with to_json, but not Vega specs, and then include the way to create Vega specs directly in Jupyter via IPython.display that you mentioned in #2817?

@mattijn mattijn closed this Jan 13, 2023
@mattijn mattijn deleted the remove-vega-v5-vegalite-v3-v4-wrappers branch January 13, 2023 20:59
@mattijn mattijn mentioned this pull request Jan 13, 2023
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.

2 participants