Repository: altair-viz/altair
Pull request #3031 (2023-04-23 20:46:48)

include view definitions for a layercharts containing a repeat + toplevel selection parameter
Fix https://github.com/altair-viz/altair/issues/2849.

- In the `LayerChart` the params of the subcharts are combined, see: https://github.com/altair-viz/altair/blob/master/altair/vegalite/v5/api.py#L2978
- Currently the `view` for these params is populated here: https://github.com/altair-viz/altair/blob/master/altair/vegalite/v5/api.py#L3272. Around here  is a loop on the `subcharts`, as far as I can see, they are not aware of being in a `RepeatChart`.
- But once we reach the `RepeatChart`, we are aware that it is a `LayerChart`, see: https://github.com/altair-viz/altair/blob/master/altair/vegalite/v5/api.py#L2664.
- At this place we also define the view names for the repeat. We can adjust the code here to accommodate for the required behavior.

With this PR we include the `spec` as argument in the function `_repeat_names()` and `_extend_view_name()`. If the instance of `spec` is `Chart`, the code functions as usual, but when `spec` is an instance of `LayerChart` we now also will enter the function `_repeat_names()`. And within the `extend_view_name()` the `view_<no>` should be placed at the end of the `view`-name instead (vs start for a non-layered repeat chart).

I included the interactive crossfilter as test, and tested it locally.

I think this works as expected, agree @ChristopherDavisUCI?

ChristopherDavisUCI: Great @mattijn, thanks!  I tried this out and looked over the code and it all makes sense.

Are parameters now working fully as desired, or are there still some open tasks on the Vega-Lite side (or the Altair side) remaining?
mattijn: Thanks for the review. I will merge this in then. I think we are feature complete for another rc2 release, but I'll have to double check.

Patch:
diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py
index 1c4bc250..e31182cd 100644
--- a/altair/vegalite/v5/api.py
+++ b/altair/vegalite/v5/api.py
@@ -2661,8 +2661,8 @@ class RepeatChart(TopLevelMixin, core.TopLevelRepeatSpec):
         _spec_as_list = [spec]
         params, _spec_as_list = _combine_subchart_params(params, _spec_as_list)
         spec = _spec_as_list[0]
-        if isinstance(spec, Chart):
-            params = _repeat_names(params, repeat)
+        if isinstance(spec, (Chart, LayerChart)):
+            params = _repeat_names(params, repeat, spec)
         super(RepeatChart, self).__init__(
             repeat=repeat,
             spec=spec,
@@ -3305,15 +3305,21 @@ def _get_repeat_strings(repeat):
     return ["".join(s) for s in itertools.product(*rcstrings)]
 
 
-def _extend_view_name(v, r):
+def _extend_view_name(v, r, spec):
     # prevent the same extension from happening more than once
-    if v.endswith("child__" + r):
-        return v
-    else:
-        return f"{v}_child__{r}"
+    if isinstance(spec, Chart):
+        if v.endswith("child__" + r):
+            return v
+        else:
+            return f"{v}_child__{r}"
+    elif isinstance(spec, LayerChart):
+        if v.startswith("child__" + r):
+            return v
+        else:
+            return f"child__{r}_{v}"
 
 
-def _repeat_names(params, repeat):
+def _repeat_names(params, repeat, spec):
     if params is Undefined:
         return params
 
@@ -3328,10 +3334,17 @@ def _repeat_names(params, repeat):
         views = []
         repeat_strings = _get_repeat_strings(repeat)
         for v in param.views:
-            if any(v.endswith(f"child__{r}") for r in repeat_strings):
-                views.append(v)
-            else:
-                views += [_extend_view_name(v, r) for r in repeat_strings]
+            if isinstance(spec, Chart):
+                if any(v.endswith(f"child__{r}") for r in repeat_strings):
+                    views.append(v)
+                else:
+                    views += [_extend_view_name(v, r, spec) for r in repeat_strings]
+            elif isinstance(spec, LayerChart):
+                if any(v.startswith(f"child__{r}") for r in repeat_strings):
+                    views.append(v)
+                else:
+                    views += [_extend_view_name(v, r, spec) for r in repeat_strings]
+
         p.views = views
         params_named.append(p)
 


Test patch:
diff --git a/tests/vegalite/v5/test_params.py b/tests/vegalite/v5/test_params.py
index 104884dd..91d29928 100644
--- a/tests/vegalite/v5/test_params.py
+++ b/tests/vegalite/v5/test_params.py
@@ -163,3 +163,36 @@ def test_selection_condition():
 
     # The else condition
     assert dct["encoding"]["size"]["value"] == 10
+
+
+def test_creation_views_params_layered_repeat_chart():
+    import altair as alt
+    from vega_datasets import data
+
+    source = alt.UrlData(data.flights_2k.url, format={"parse": {"date": "date"}})
+
+    brush = alt.selection_interval(encodings=["x"])
+
+    # Define the base chart, with the common parts of the
+    # background and highlights
+    base = (
+        alt.Chart(width=160, height=130)
+        .mark_bar()
+        .encode(x=alt.X(alt.repeat("column")).bin(maxbins=20), y="count()")
+    )
+
+    # gray background with selection
+    background = base.encode(color=alt.value("#ddd")).add_params(brush)
+
+    # blue highlights on the transformed data
+    highlight = base.transform_filter(brush)
+
+    # layer the two charts & repeat
+    c = (
+        alt.layer(background, highlight, data=source)
+        .transform_calculate("time", "hours(datum.date)")
+        .repeat(column=["distance", "delay", "time"])
+    )
+
+    dct = c.to_dict()
+    assert "child__column_distance_view_" in dct["params"][0]["views"][0]