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

Compatability with the data server transformer #31

Open
joelostblom opened this issue Nov 23, 2022 · 5 comments
Open

Compatability with the data server transformer #31

joelostblom opened this issue Nov 23, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@joelostblom
Copy link

I noticed that enabling the data server transformer causes vl convert to hang indefinitely. Is this something that is fixable or is vl-convert meant to be used with vega-fusion instead? Since we are planning to have vl-convert take precedence over altair_saver, it would be great if it also worked with the data server transformer like altair_saver does to ensure full backwards compatibility.

Example that hangs for me on vl-convert 0.5.0 and altair 4.2.0, python 3.10.x

import pandas as pd
import altair as alt

alt.data_transformers.enable('data_server')
source = pd.DataFrame({
    'a': ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I'],
    'b': [28, 55, 43, 91, 81, 53, 19, 87, 52]
})
chart = alt.Chart(source).mark_bar().encode(
    x='a',
    y='b'
)


# Saving the chart
import vl_convert as vlc

png_data = vlc.vegalite_to_png(chart.to_dict())
with open("chart.png", "wb") as f:
    f.write(png_data)
@jonmmease jonmmease added the bug Something isn't working label Nov 23, 2022
@jonmmease
Copy link
Collaborator

Thanks for the report @joelostblom. vl-convert doesn't assume anything about using VegaFusion, and it should be able to load data from the URL served by the data server transformer so I'm not sure yet what's going on.

@jonmmease
Copy link
Collaborator

Ok, I spent some time looking at this. It looks like a deadlock is being created between the tornado event loop that the Altair data server uses to serve files in the background, and the async runtime event loop that Deno is running on. I tried a couple of things, but this might not be an issue we can solve in the near term.

So on the Altair side, maybe we should consider swapping the default back to altair_saver for v5. We could also probably detect that altair data server is active and raise an informative error message if the vl-convert logic path is invoked in this situation.

@jonmmease
Copy link
Collaborator

Actually, I think we could just disable data transformers all together when calling chart.to_dict for the purpose of image export. I think it should always be ok to inline all of the data.

@joelostblom
Copy link
Author

Hmmm, I very much want to keep vl-convert the default for the next Altair version. When I use altair_saver in class there are always issues for students, and this year I couldn't work around them so I made a function that wrapped vl-convert and they could use, which has been seamless for most of them!

I discovered that the data transformers can be used as context managers, so what do you think of just wrapping the calls to vl-convert in the Altair code base like this:

with alt.data_transformers.enable('default') and alt.data_transformers.disable_max_rows():
    png_data = vlc.vegalite_to_png(chart.to_dict())
    with open("chart.png", "wb") as f:
        f.write(png_data)

Do you foresee any issues with that?

@jonmmease
Copy link
Collaborator

Great to hear vl-convert worked well for your students!

Yeah, this is exactly what I was thinking. Using the default data transformer as a context manager looks like a great approach. Feel free to go ahead and add that to Altair if it's easy for you, but I can take care of it next week otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants