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

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

Merged
merged 2 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions altair/vegalite/v5/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
34 changes: 34 additions & 0 deletions tests/test_examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down