From b27b829b03cf2bbf01c7970e02f020de2a741f73 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Sun, 26 Mar 2023 12:41:39 -0700 Subject: [PATCH] Display more helpful error message when two fields strings are used in condition (#2979) * Raise helpful error if a field is used in both branches of a condition * Add test * Add changelog entry * Blacken * Blacken more * Improve error message wording * Apply suggestions from code review Co-authored-by: Mattijn van Hoek --------- Co-authored-by: Mattijn van Hoek --- altair/vegalite/v5/api.py | 9 +++++++-- doc/releases/changes.rst | 1 + tests/utils/test_schemapi.py | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 8af926602..a1b3388e8 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -773,8 +773,13 @@ def condition(predicate, if_true, if_false, **kwargs): # dict in the appropriate schema if_true = if_true.to_dict() elif isinstance(if_true, str): - if_true = utils.parse_shorthand(if_true) - if_true.update(kwargs) + if isinstance(if_false, str): + raise ValueError( + "A field cannot be used for both the `if_true` and `if_false` values of a condition. One of them has to specify a `value` or `datum` definition." + ) + else: + if_true = utils.parse_shorthand(if_true) + if_true.update(kwargs) condition.update(if_true) if isinstance(if_false, core.SchemaBase): diff --git a/doc/releases/changes.rst b/doc/releases/changes.rst index 9dda323cf..14168e59b 100644 --- a/doc/releases/changes.rst +++ b/doc/releases/changes.rst @@ -24,6 +24,7 @@ Enhancements - Substantially improved error handling. Both in terms of finding the more relevant error (#2842), and in terms of improving the formatting and clarity of the error messages (#2824, #2568). - Include experimental support for the DataFrame Interchange Protocol (through ``__dataframe__`` attribute). This requires ``pyarrow>=11.0.0`` (#2888). - Support data type inference for columns with special characters (#2905). +- Changed error message to be more informative when two field strings are using in a condition (#2979). Grammar Changes ~~~~~~~~~~~~~~~ diff --git a/tests/utils/test_schemapi.py b/tests/utils/test_schemapi.py index 35f14ce25..8bc11b09b 100644 --- a/tests/utils/test_schemapi.py +++ b/tests/utils/test_schemapi.py @@ -610,6 +610,20 @@ def test_chart_validation_errors(chart_func, expected_error_message): chart.to_dict() +def test_multiple_field_strings_in_condition(): + selection = alt.selection_point() + expected_error_message = "A field cannot be used for both the `if_true` and `if_false` values of a condition. One of them has to specify a `value` or `datum` definition." + with pytest.raises(ValueError, match=expected_error_message): + ( + alt.Chart(data.cars()) + .mark_circle() + .add_params(selection) + .encode( + color=alt.condition(selection, "Origin", "Origin"), + ) + ).to_dict() + + def test_serialize_numpy_types(): m = MySchema( a={"date": np.datetime64("2019-01-01")},