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

Add support to transformed_data for reconstructed (from_dict) charts #354

Open
binste opened this issue Jul 7, 2023 · 4 comments
Open
Labels
bug Something isn't working

Comments

@binste
Copy link

binste commented Jul 7, 2023

Relates to #313. For layered and multi-view charts, Altair takes a bit of a shortcut and does not use the exact classes which map to the Vega-Lite schema but instead uses the API convenience classes such as alt.Chart, alt.LayerChart, etc. For example for the layered chart below, the elements of layer are alt.Chart instances:

import altair as alt
import vegafusion as vf
from vega_datasets import data

source = data.wheat()

bar = alt.Chart(source).mark_bar().encode(
    x='year:O',
    y='wheat:Q'
)

rule = alt.Chart(source).mark_rule(color='red').encode(
    y='mean(wheat):Q'
)

layer_chart = (bar + rule).properties(width=600)
type(layer_chart.layer[0])  # altair.vegalite.v5.api.Chart

However, when reconstructing this chart from a dictionary, the layers are of type UnitSpec which is the correct class following the VL schema:

reconstructed_layer_chart = alt.Chart.from_dict(layer_chart.to_dict())
type(reconstructed_layer_chart.layer[0])  # altair.vegalite.v5.schema.core.UnitSpec

When calling transformed_data on the reconstructed chart, it fails as the isinstance checks do not accept UnitSpec as an equivalent to alt.Chart although it should be:

vf.transformed_data(reconstructed_layer_chart)
ValueError: transformed_data accepts an instance of Chart, FacetChart, LayerChart, HConcatChart, VConcatChart, or ConcatChart
Received value of type: <class 'altair.vegalite.v5.schema.core.UnitSpec'>

All classes which I think can be treated as equivalent to the first one mentioned in every bullet:

  • Chart: TopLevelUnitSpec (parent class of Chart), FacetedUnitSpec, UnitSpec, UnitSpecWithFrame, NonNormalizedSpec (used in concat charts for chart objects in e.g. hconcat attribute)
  • LayerChart: TopLevelLayerSpec (parent class of LayerChart), LayerSpec
  • RepeatChart: TopLevelRepeatSpec (parent class of RepeatChart), RepeatSpec
  • ConcatChart: TopLevelConcatSpec (parent class of ConcatChart), ConcatSpecGenericSpec
  • HConcatChart: TopLevelHConcatSpec (parent class of HConcatChart), HConcatSpecGenericSpec
  • VConcatChart: TopLevelVConcatSpec (parent class of VConcatChart), VConcatSpecGenericSpec
  • FacetChart: TopLevelFacetSpec (parent class of FacetChart), FacetSpec

Spec classes which I'm not sure about but we can probably ignore as repeat charts are not supported yet by transformed_data anyway:

  • LayerRepeatSpec
  • NonLayerRepeatSpec
  • GenericUnitSpecEncodingAnyMark

Just a reminder that the same changes would need to be applied to https://github.com/altair-viz/altair/blob/master/altair/utils/_transformed_data.py.

Btw, my use case for the above is that a frontend sends the chart specifications as a JSON to a backend API which executes vf.transformed_data(alt.Chart.from_dict(spec)) and then returns the extracted data as an Excel file to the user of the web application.

@jonmmease
Copy link
Collaborator

Thanks for the report @binste, I think this all makes sense. And now that chart.transformed_data is in Altair master, we need to fix it there as well!

@jonmmease jonmmease added the bug Something isn't working label Jul 7, 2023
@binste
Copy link
Author

binste commented Jul 7, 2023

I don't have a working development setup for VegaFusion right now. Is it easy to set it up to work purely on the Python part without Rust or do I need to compile the Rust code anyway? I assume the later. Let me know if I should land a hand on implementing the changes in Altair.

@jonmmease
Copy link
Collaborator

Let me know if I should land a hand on implementing the changes in Altair.

You can develop the Python part of VegaFusion on its own. But if you want to work on fixing it in the Altair implementation of chart.transformed_data (which doesn't use the vf.transformed_data implementation), I could pretty easily copy the fixes into the VegaFusion implementation afterward.

@binste
Copy link
Author

binste commented Jul 7, 2023

Great, I'll work on the Altair implementation probably early next week.

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