Fix bug in reconstructing layered charts with from_json/from_dict (#3068)

* Fix for when subcharts do not have layer_props

* Test the to_json and from_json methods using all examples in the gallery

Author: binste

altair/vegalite/v5/api.py

@@ -3395,9 +3395,12 @@ def remove_prop(subchart, prop):
             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}")

tests/test_examples.py

@@ -3,6 +3,7 @@
 
 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