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

Move "inline" save functionality from altair_saver into altair? #2765

Closed
joelostblom opened this issue Dec 19, 2022 · 5 comments
Closed

Move "inline" save functionality from altair_saver into altair? #2765

joelostblom opened this issue Dec 19, 2022 · 5 comments
Assignees
Labels

Comments

@joelostblom
Copy link
Contributor

Since we now have vl-convert as the main saving functionality, it could make sense to move some functionality from altair_saver that is not covered by vl-convert into the altair library itself. At least functionality pertaining to saving HTML files, since this operation is already possible in altair without any additional library.

For this issue I am mostly thinking of moving the "inline" parameter into core from saver as mentioned in vega/vl-convert#33 (comment). It seems like there is only string manipulation involved in using the "inline" parameter https://github.com/altair-viz/altair_saver/blob/f5c11bb94562dee73b614255dc1122f948971479/altair_saver/savers/_html.py

@jonmmease
Copy link
Contributor

I'll pick this up, feel free to assign me.

@jonmmease
Copy link
Contributor

It looks like the inline Vega/Vega-Lite source is currently pulled from altair_viewer. Does that still sound like a good idea? Altair viewer seems like a still useful package, but does it make sense to require it in order for the chart.save("file.html", inline=True) to work?

@joelostblom
Copy link
Contributor Author

joelostblom commented Jan 5, 2023

Hmm, I feel a bit unqualified to make the call of whether we should move that code into altair code. From a user perspective I would prefer if all saving functionality was covered by the core library and the saving-related packages such as vl-convert. Although it does seems like quite a lot of code.

Do you think including that part in altair would facilitate implementation of #2797?

@jonmmease
Copy link
Contributor

Looks like it would be about 1MB (uncompressed) to add Vega, Vega-Lite, and Vega-Embed to the Altair package. Here's a PR that follow altair_saver by pulling the bundles from altair_viewer: #2807.

Also happy to look at bringing the bundles over to Altair if that's the direction we want to go.

@joelostblom
Copy link
Contributor Author

Implemented in #2807

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

No branches or pull requests

3 participants