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

Deprecate to_html()? Or make it work with Vegafusion? #3396

Open
joelostblom opened this issue Apr 7, 2024 · 5 comments
Open

Deprecate to_html()? Or make it work with Vegafusion? #3396

joelostblom opened this issue Apr 7, 2024 · 5 comments
Assignees
Labels

Comments

@joelostblom
Copy link
Contributor

What happened?

I accidentally mixed up to_html() and save and noticed that when using to_html with the vegafusion data_transformers is enabled, I run into this error:

ValueError: When the "vegafusion" data transformer is enabled, the 
to_dict() and to_json() chart methods must be called with format="vega". 
For example: 
    >>> chart.to_dict(format="vega")
    >>> chart.to_json(format="vega")
import altair as alt
import pandas as pd

alt.data_transformers.enable('vegafusion')

source = pd.DataFrame({
    'a': ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I'],
    'b': [28, 55, 43, 91, 81, 53, 19, 87, 52]
})

alt.Chart(source).mark_bar().encode(
    x='a',
    y='b'
).to_html()

What would you like to happen instead?

Instead of hardcoding to_dict() without args and mode='vegalite', we could allow to pass arguments for these parameters in the to_html() method. The mode could just be named mode and the dict option could either just be format or dict_kwds similar to what we have for json_kwds. It's possible to override the mode via embed_options but I think an explicit argument would be clearer.

Another option would be to deprecate it. It has been mention in the past that it is no longer used, but not sure if it was reincarnated after that or just never removed #420 (comment)

Which version of Altair are you using?

main

@joelostblom joelostblom added the bug label Apr 7, 2024
@jonmmease jonmmease self-assigned this Apr 7, 2024
@jonmmease
Copy link
Contributor

I think we can dynamically choose the mode based on whether VegaFusion is enabled. The easiest thing might be to reimplement to_html, in terms of save, which already does this.

But this does raise the question of why we have this method in the first place!

@mattijn
Copy link
Contributor

mattijn commented Apr 8, 2024

With a few more code reincarnations and a bit of generation, we can resolve this related issue #3189.

@mattijn
Copy link
Contributor

mattijn commented Apr 8, 2024

In line with what I said in #3189, I would be in favor of keeping this method and make it works with vegafusion.

@joelostblom
Copy link
Contributor Author

Sounds good to me!

@NickCrews
Copy link
Contributor

I use .to_html() to save multiple charts into a template that holds multiple charts in different tabs, so it would be great for me if we could keep it around!

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

4 participants