Repository: altair-viz/altair (python)

Issue:
JSON to Chart failure in v5.0.0
As part of a larger application, we're moving chart content around in JSON format via `Chart.to_json` and `Chart.from_json`. However, I'm encountering an error after updating to Altair v5.0.0. An example that used to work previously - based on the layered chart in https://altair-viz.github.io/altair-tutorial/notebooks/09-Geographic-plots.html

```python
import altair as alt
from vega_datasets import data

states = alt.topo_feature(data.us_10m.url, feature='states')
airports = data.airports()

background = alt.Chart(states).mark_geoshape(
    fill='lightgray',
    stroke='white'
).project('albersUsa').properties(
    width=500,
    height=300
)

points = alt.Chart(airports).mark_circle().encode(
    longitude='longitude:Q',
    latitude='latitude:Q',
    size=alt.value(10),
    tooltip='name'
)

chart = background + points

json = chart.to_json()
chart = alt.Chart.from_json(json)
```

In v4.2.2 this runs without any errors. In v5.0.0 this results in

```
Traceback (most recent call last):
  File "altair_scratchpad.py", line 25, in <module>
    chart = alt.Chart.from_json(json)
  File "C:\Users\romesh\miniconda3\envs\atomica38\lib\site-packages\altair\utils\schemapi.py", line 914, in from_json
    return cls.from_dict(dct, validate=validate)
  File "C:\Users\romesh\miniconda3\envs\atomica38\lib\site-packages\altair\vegalite\v5\api.py", line 2497, in from_dict
    return class_.from_dict(dct, validate=validate)  # type: ignore[attr-defined]
  File "C:\Users\romesh\miniconda3\envs\atomica38\lib\site-packages\altair\utils\schemapi.py", line 893, in from_dict
    return converter.from_dict(dct, cls)
  File "C:\Users\romesh\miniconda3\envs\atomica38\lib\site-packages\altair\utils\schemapi.py", line 1058, in from_dict
    return cls(**kwds)
  File "C:\Users\romesh\miniconda3\envs\atomica38\lib\site-packages\altair\vegalite\v5\api.py", line 2982, in __init__
    combined_dict, self.layer = _remove_layer_props(self, self.layer, layer_props)
  File "C:\Users\romesh\miniconda3\envs\atomica38\lib\site-packages\altair\vegalite\v5\api.py", line 3393, in _remove_layer_props
    if all(c[prop] is Undefined or c[prop] == chart[prop] for c in subcharts):
  File "C:\Users\romesh\miniconda3\envs\atomica38\lib\site-packages\altair\vegalite\v5\api.py", line 3393, in <genexpr>
    if all(c[prop] is Undefined or c[prop] == chart[prop] for c in subcharts):
  File "C:\Users\romesh\miniconda3\envs\atomica38\lib\site-packages\altair\utils\schemapi.py", line 711, in __getitem__
    return self._kwds[item]
KeyError: 'height'
```


Pull request description:
Fix bug in reconstructing layered charts with from_json/from_dict
Fixes #3067. When reconstructing a layered chart which has `height`or `width` defined on the top-level with `from_dict`, the code so far checked that the layers have that property set to `Undefined` or to the same value as the layered chart itself. However, it's often the case that the layers don't have the property set at all -> This fix handles those cases.

I also added a test to run a to_json and from_json roundtrip for all examples in the gallery so that we can hopefully catch future regressions to this functionality. I checked that these tests would have caught the bug reported in #3067.

@mattijn Would be great if this fix can still make it into 5.0.1 in case you or someone else has the time to review it.

Patch:
diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py
index e9aa439c..ed449bca 100644
--- a/altair/vegalite/v5/api.py
+++ b/altair/vegalite/v5/api.py
@@ -3395,9 +3395,12 @@ def _remove_layer_props(chart, subcharts, layer_props):
             else:
                 raise ValueError(f"There are inconsistent values {values} for {prop}")
         else:
-            # Top level has this prop; subchart props must be either
-            # Undefined or identical to proceed.
-            if all(c[prop] is Undefined or c[prop] == chart[prop] for c in subcharts):
+            # Top level has this prop; subchart must either not have the prop
+            # or it must be Undefined or identical to proceed.
+            if all(
+                getattr(c, prop, Undefined) is Undefined or c[prop] == chart[prop]
+                for c in subcharts
+            ):
                 output_dict[prop] = chart[prop]
             else:
                 raise ValueError(f"There are inconsistent values {values} for {prop}")


Test patch:
diff --git a/tests/examples_arguments_syntax/natural_disasters.py b/tests/examples_arguments_syntax/natural_disasters.py
index 031c1e98..890580a6 100644
--- a/tests/examples_arguments_syntax/natural_disasters.py
+++ b/tests/examples_arguments_syntax/natural_disasters.py
@@ -36,7 +36,7 @@ alt.Chart(source).transform_filter(
 ).properties(
     width=450,
     height=320,
-    title=alt.TitleParams(
+    title=alt.Title(
         text="Global Deaths from Natural Disasters (1900-2017)",
         subtitle="The size of the bubble represents the total death count per year, by type of disaster",
         anchor='start'
diff --git a/tests/examples_arguments_syntax/scatter_linked_table.py b/tests/examples_arguments_syntax/scatter_linked_table.py
index c31c4a9c..93777b75 100644
--- a/tests/examples_arguments_syntax/scatter_linked_table.py
+++ b/tests/examples_arguments_syntax/scatter_linked_table.py
@@ -1,7 +1,7 @@
 """
 Brushing Scatter Plot to Show Data on a Table
 ---------------------------------------------
-A scatter plot of the cars dataset, with data tables for horsepower, MPG, and origin. 
+A scatter plot of the cars dataset, with data tables for horsepower, MPG, and origin.
 The tables update to reflect the selection on the scatter plot.
 """
 # category: scatter plots
@@ -18,26 +18,30 @@ brush = alt.selection_interval()
 points = alt.Chart(source).mark_point().encode(
     x='Horsepower:Q',
     y='Miles_per_Gallon:Q',
-    color=alt.condition(brush, 'Cylinders:O', alt.value('grey'))
+    color=alt.condition(brush, alt.value('steelblue'), alt.value('grey'))
 ).add_params(brush)
 
 # Base chart for data tables
-ranked_text = alt.Chart(source).mark_text().encode(
-    y=alt.Y('row_number:O',axis=None)
-).transform_window(
-    row_number='row_number()'
+ranked_text = alt.Chart(source).mark_text(align='right').encode(
+    y=alt.Y('row_number:O', axis=None)
 ).transform_filter(
     brush
 ).transform_window(
-    rank='rank(row_number)'
+    row_number='row_number()'
 ).transform_filter(
-    alt.datum.rank<20
+    alt.datum.row_number < 15
 )
 
 # Data Tables
-horsepower = ranked_text.encode(text='Horsepower:N').properties(title='Horsepower')
-mpg = ranked_text.encode(text='Miles_per_Gallon:N').properties(title='MPG')
-origin = ranked_text.encode(text='Origin:N').properties(title='Origin')
+horsepower = ranked_text.encode(text='Horsepower:N').properties(
+    title=alt.Title(text='Horsepower', align='right')
+)
+mpg = ranked_text.encode(text='Miles_per_Gallon:N').properties(
+    title=alt.Title(text='MPG', align='right')
+)
+origin = ranked_text.encode(text='Origin:N').properties(
+    title=alt.Title(text='Origin', align='right')
+)
 text = alt.hconcat(horsepower, mpg, origin) # Combine data tables
 
 # Build chart
@@ -46,4 +50,6 @@ alt.hconcat(
     text
 ).resolve_legend(
     color="independent"
+).configure_view(
+    stroke=None
 )
diff --git a/tests/examples_methods_syntax/natural_disasters.py b/tests/examples_methods_syntax/natural_disasters.py
index 31b50a15..cbc5a0d6 100644
--- a/tests/examples_methods_syntax/natural_disasters.py
+++ b/tests/examples_methods_syntax/natural_disasters.py
@@ -36,7 +36,7 @@ alt.Chart(source).transform_filter(
 ).properties(
     width=450,
     height=320,
-    title=alt.TitleParams(
+    title=alt.Title(
         text="Global Deaths from Natural Disasters (1900-2017)",
         subtitle="The size of the bubble represents the total death count per year, by type of disaster",
         anchor='start'
diff --git a/tests/examples_methods_syntax/scatter_linked_table.py b/tests/examples_methods_syntax/scatter_linked_table.py
index d2295e27..99979e07 100644
--- a/tests/examples_methods_syntax/scatter_linked_table.py
+++ b/tests/examples_methods_syntax/scatter_linked_table.py
@@ -29,18 +29,18 @@ ranked_text = alt.Chart(source).mark_text(align='right').encode(
 ).transform_window(
     row_number='row_number()'
 ).transform_filter(
-    'datum.row_number < 15'
+    alt.datum.row_number < 15
 )
 
 # Data Tables
 horsepower = ranked_text.encode(text='Horsepower:N').properties(
-    title=alt.TitleParams(text='Horsepower', align='right')
+    title=alt.Title(text='Horsepower', align='right')
 )
 mpg = ranked_text.encode(text='Miles_per_Gallon:N').properties(
-    title=alt.TitleParams(text='MPG', align='right')
+    title=alt.Title(text='MPG', align='right')
 )
 origin = ranked_text.encode(text='Origin:N').properties(
-    title=alt.TitleParams(text='Origin', align='right')
+    title=alt.Title(text='Origin', align='right')
 )
 text = alt.hconcat(horsepower, mpg, origin) # Combine data tables
 
diff --git a/tests/test_examples.py b/tests/test_examples.py
index 578f3830..da580806 100644
--- a/tests/test_examples.py
+++ b/tests/test_examples.py
@@ -3,6 +3,7 @@ import pkgutil
 
 import pytest
 
+import altair as alt
 from altair.utils.execeval import eval_block
 from tests import examples_arguments_syntax
 from tests import examples_methods_syntax
@@ -48,6 +49,39 @@ def test_render_examples_to_chart(syntax_module):
             ) from err
 
 
+@pytest.mark.parametrize(
+    "syntax_module", [examples_arguments_syntax, examples_methods_syntax]
+)
+def test_from_and_to_json_roundtrip(syntax_module):
+    """Tests if the to_json and from_json (and by extension to_dict and from_dict)
+    work for all examples in the Example Gallery.
+    """
+    for filename in iter_examples_filenames(syntax_module):
+        source = pkgutil.get_data(syntax_module.__name__, filename)
+        chart = eval_block(source)
+
+        if chart is None:
+            raise ValueError(
+                f"Example file {filename} should define chart in its final "
+                "statement."
+            )
+
+        try:
+            first_json = chart.to_json()
+            reconstructed_chart = alt.Chart.from_json(first_json)
+            # As the chart objects are not
+            # necessarily the same - they could use different objects to encode the same
+            # information - we do not test for equality of the chart objects, but rather
+            # for equality of the json strings.
+            second_json = reconstructed_chart.to_json()
+            assert first_json == second_json
+        except Exception as err:
+            raise AssertionError(
+                f"Example file {filename} raised an exception when "
+                f"doing a json conversion roundtrip: {err}"
+            ) from err
+
+
 # We do not apply the save_engine mark to this test. This mark is used in
 # the build GitHub Action workflow to select the tests which should be rerun
 # with some of the saving engines uninstalled. This would not make sense for this test