Repository: altair-viz/altair

Issue:
Altair selections resolve incorrectly as scale domain values.
I am observing this issue in Altair 2.x. Apologies if this issue has already been addressed for v3!

The following code for an overview + detail plot fails:

```python
brush = alt.selection_interval(encodings=['x']);

base = alt.Chart().mark_area().encode(
    x=alt.X('date:T', title=None),
    y='price:Q'
).properties(
    width=700
)
  
alt.vconcat(
    base.encode(alt.X('date:T', title=None, scale=alt.Scale(domain=brush))),
    base.add_selection(brush).properties(height=60),
    data='https://vega.github.io/vega-datasets/data/sp500.csv'
)
```

The errors indicate that Altair is expanding the brush to the full selection definition (what you would use for `add_selection`) rather than generating a selection reference. If I add `name='brush'` to the selection definition and then replace `domain=brush` with `domain={'selection': 'brush'}`, the example works.

Discussion:

Patch:
diff --git a/CHANGES.md b/CHANGES.md
index e0b1d71b..4f0dae9f 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,8 +1,8 @@
 # Altair Change Log
 
-## Version 3.0.0rc1 (prerelease)
+## Version 3.0.0 (unreleased)
 
-Update to Vega-Lite 3.0 and Vega 5.0 & support all new features. See
+Update to Vega-Lite 3.2 and Vega 5.0 & support all new features. See
 https://github.com/vega/vega-lite/releases/tag/v3.0.0 for Vega-Lite
 feature lists.
 
@@ -14,6 +14,26 @@ feature lists.
 - tooltips enabled by default. Pass encoding ``tooltip=None`` to disable.
 - new ``columns`` keyword that allows wrapped faceting and concatenation.
 
+### Backward-incompatible changes:
+
+- selections can be bound to scales directly; i.e. you can use
+  ```
+  alt.Scale(domain=selection)
+  ```
+  instead of
+  ```
+  alt.Scale(domain=selection.ref())
+  ```
+
+- selections can no longer be passed directly as chart properties; i.e. you should use
+  ```
+  chart.add_selection(selection1, selection2)
+  ```
+  instead of
+  ```
+  chart.properties(selection=selection1 + selection2)
+  ```
+
 ## Version 2.4.1 (Released February 21, 2019)
 
 ### Enhancements
diff --git a/altair/examples/interactive_cross_highlight.py b/altair/examples/interactive_cross_highlight.py
index 07a7d2a1..1ba80cf2 100644
--- a/altair/examples/interactive_cross_highlight.py
+++ b/altair/examples/interactive_cross_highlight.py
@@ -36,10 +36,9 @@ bar = alt.Chart(source).mark_bar().encode(
     y='count()',
     color=alt.condition(pts, alt.ColorValue("steelblue"), alt.ColorValue("grey"))
 ).properties(
-    selection=pts,
     width=550,
     height=200
-)
+).add_selection(pts)
 
 alt.vconcat(
     rect + circ,
diff --git a/altair/examples/interactive_layered_crossfilter.py b/altair/examples/interactive_layered_crossfilter.py
index 21d67a47..248103d0 100644
--- a/altair/examples/interactive_layered_crossfilter.py
+++ b/altair/examples/interactive_layered_crossfilter.py
@@ -27,7 +27,7 @@ base = alt.Chart().mark_bar().encode(
 )
 
 # blue background with selection
-background = base.properties(selection=brush)
+background = base.add_selection(brush)
 
 # yellow highlights on the transformed data
 highlight = base.encode(
diff --git a/altair/examples/interval_selection.py b/altair/examples/interval_selection.py
index 941f0295..a5ba1209 100644
--- a/altair/examples/interval_selection.py
+++ b/altair/examples/interval_selection.py
@@ -15,7 +15,7 @@ source = data.sp500.url
 brush = alt.selection(type='interval', encodings=['x'])
 
 upper = alt.Chart().mark_area().encode(
-    alt.X('date:T', scale={'domain': brush.ref()}),
+    alt.X('date:T', scale=alt.Scale(domain=brush)),
     y='price:Q'
 ).properties(
     width=600,
diff --git a/altair/examples/scatter_with_histogram.py b/altair/examples/scatter_with_histogram.py
index 8d7a2a27..163ede81 100644
--- a/altair/examples/scatter_with_histogram.py
+++ b/altair/examples/scatter_with_histogram.py
@@ -32,7 +32,7 @@ points = alt.Chart().mark_point(filled=True, color="black").encode(
     x='x',
     y='y'
 ).transform_filter(
-    pts.ref()
+    pts
 ).properties(
     width=300,
     height=300
@@ -44,10 +44,9 @@ mag = alt.Chart().mark_bar().encode(
     y="count()",
     color=alt.condition(pts, alt.value("black"), alt.value("lightgray"))
 ).properties(
-    selection=pts,
     width=300,
     height=300
-)
+).add_selection(pts)
 
 # build the chart:
 alt.hconcat(
diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py
index 18f96f6d..63cecaf7 100644
--- a/altair/utils/schemapi.py
+++ b/altair/utils/schemapi.py
@@ -243,6 +243,8 @@ class SchemaBase(object):
             elif isinstance(val, dict):
                 return {k: _todict(v) for k, v in val.items()
                         if v is not Undefined}
+            elif hasattr(val, 'to_dict'):
+                return val.to_dict()
             else:
                 return val
 
diff --git a/altair/vegalite/v2/api.py b/altair/vegalite/v2/api.py
index 83fbd7ee..2a97f0d2 100644
--- a/altair/vegalite/v2/api.py
+++ b/altair/vegalite/v2/api.py
@@ -197,7 +197,7 @@ class NamedSelection(SelectionMapping):
 
         Examples
         --------
-        >>> import altair as alt
+        >>> import altair.vegalite.v2 as alt
         >>> sel = alt.selection_interval(name='interval')
         >>> sel.ref()
         {'selection': 'interval'}
@@ -685,7 +685,7 @@ class TopLevelMixin(mixins.ConfigMethodMixin):
         The aggregate transform allows you to specify transforms directly using
         the same shorthand syntax as used in encodings:
 
-        >>> import altair as alt
+        >>> import altair.vegalite.v2 as alt
         >>> chart1 = alt.Chart().transform_aggregate(
         ...     mean_acc='mean(Acceleration)',
         ...     groupby=['Origin']
@@ -752,7 +752,7 @@ class TopLevelMixin(mixins.ConfigMethodMixin):
 
         Examples
         --------
-        >>> import altair as alt
+        >>> import altair.vegalite.v2 as alt
         >>> chart = alt.Chart().transform_bin("x_binned", "x")
         >>> chart.transform[0]
         BinTransform({
@@ -807,7 +807,7 @@ class TopLevelMixin(mixins.ConfigMethodMixin):
 
         Examples
         --------
-        >>> import altair as alt
+        >>> import altair.vegalite.v2 as alt
         >>> from altair import datum, expr
 
         >>> chart = alt.Chart().transform_calculate(y = 2 * expr.sin(datum.x))
@@ -941,7 +941,7 @@ class TopLevelMixin(mixins.ConfigMethodMixin):
 
         Examples
         --------
-        >>> import altair as alt
+        >>> import altair.vegalite.v2 as alt
         >>> from altair import datum, expr
 
         >>> chart = alt.Chart().transform_timeunit(month='month(date)')
@@ -1041,7 +1041,7 @@ class TopLevelMixin(mixins.ConfigMethodMixin):
         --------
         A cumulative line chart
 
-        >>> import altair as alt
+        >>> import altair.vegalite.v2 as alt
         >>> import numpy as np
         >>> import pandas as pd
         >>> data = pd.DataFrame({'x': np.arange(100),
diff --git a/altair/vegalite/v3/api.py b/altair/vegalite/v3/api.py
index 50b0b7c6..58039645 100644
--- a/altair/vegalite/v3/api.py
+++ b/altair/vegalite/v3/api.py
@@ -150,82 +150,36 @@ def _get_channels_mapping():
 
 # -------------------------------------------------------------------------
 # Tools for working with selections
-class SelectionMapping(core.VegaLiteSchema):
-    """A mapping of selection names to selection definitions.
+class Selection(object):
+    """A Selection object"""
+    _counter = 1
 
-    This is designed to match the schema of the "selection" property of
-    top-level objects.
-    """
-    _schema = {
-        'type': 'object',
-        'additionalPropeties': {'$ref': '#/definitions/SelectionDef'}
-    }
-    _rootschema = core.Root._schema
-
-    def __add__(self, other):
-        if isinstance(other, SelectionMapping):
-            copy = self.copy()
-            copy._kwds.update(other._kwds)
-            return copy
-        else:
-            return NotImplemented
-
-    def __iadd__(self, other):
-        if isinstance(other, SelectionMapping):
-            self._kwds.update(other._kwds)
-            return self
-        else:
-            return NotImplemented
-
-
-class NamedSelection(SelectionMapping):
-    """A SelectionMapping with a single named selection item"""
-    _schema = {
-        'type': 'object',
-        'additionalPropeties': {'$ref': '#/definitions/SelectionDef'},
-        'minProperties': 1, 'maxProperties': 1
-    }
-    _rootschema = core.Root._schema
-
-    def _get_name(self):
-        if len(self._kwds) != 1:
-            raise ValueError("NamedSelection has multiple properties")
-        return next(iter(self._kwds))
+    def __init__(self, name, selection):
+        if name is None:
+            name = "selector{:03d}".format(self._counter)
+            self._counter += 1
+        self.name = name
+        self.selection = selection
 
     def ref(self):
-        """Return a selection reference to this object
-
-        Examples
-        --------
-        >>> import altair as alt
-        >>> sel = alt.selection_interval(name='interval')
-        >>> sel.ref()
-        {'selection': 'interval'}
+        return {'selection': self.name}
 
-        """
-        return {"selection": self._get_name()}
+    def to_dict(self):
+        return {'selection': self.name}
 
     def __invert__(self):
-        return core.SelectionNot(**{'not': self._get_name()})
+        return core.SelectionNot(**{'not': self.name})
 
     def __and__(self, other):
-        if isinstance(other, NamedSelection):
-            other = other._get_name()
-        return core.SelectionAnd(**{'and': [self._get_name(), other]})
+        if isinstance(other, Selection):
+            other = other.name
+        return core.SelectionAnd(**{'and': [self.name, other]})
 
     def __or__(self, other):
-        if isinstance(other, NamedSelection):
-            other = other._get_name()
-        return core.SelectionOr(**{'or': [self._get_name(), other]})
+        if isinstance(other, Selection):
+            other = other.name
+        return core.SelectionOr(**{'or': [self.name, other]})
 
-    def __add__(self, other):
-        copy = SelectionMapping(**self._kwds)
-        copy += other
-        return copy
-
-    def __iadd__(self, other):
-        # this will be delegated to SelectionMapping
-        return NotImplemented
 
 # ------------------------------------------------------------------------
 # Top-Level Functions
@@ -251,15 +205,11 @@ def selection(name=None, type=Undefined, **kwds):
 
     Returns
     -------
-    selection: NamedSelection
+    selection: Selection
         The selection object that can be used in chart creation.
     """
-    if name is None:
-        name = "selector{:03d}".format(selection.counter)
-        selection.counter += 1
-    return NamedSelection(**{name: core.SelectionDef(type=type, **kwds)})
+    return Selection(name, core.SelectionDef(type=type, **kwds))
 
-selection.counter = 1
 
 @utils.use_signature(core.IntervalSelection)
 def selection_interval(**kwargs):
@@ -314,7 +264,7 @@ def condition(predicate, if_true, if_false, **kwargs):
 
     Parameters
     ----------
-    predicate: NamedSelection, LogicalOperandPredicate, expr.Expression, dict, or string
+    predicate: Selection, LogicalOperandPredicate, expr.Expression, dict, or string
         the selection predicate or test predicate for the condition.
         if a string is passed, it will be treated as a test operand.
     if_true:
@@ -339,8 +289,8 @@ def condition(predicate, if_true, if_false, **kwargs):
                        core.FieldGTPredicate, core.FieldLTEPredicate,
                        core.FieldGTEPredicate, core.SelectionPredicate)
 
-    if isinstance(predicate, NamedSelection):
-        condition = {'selection': predicate._get_name()}
+    if isinstance(predicate, Selection):
+        condition = {'selection': predicate.name}
     elif isinstance(predicate, selection_predicates):
         condition = {'selection': predicate}
     elif isinstance(predicate, test_predicates):
@@ -563,16 +513,15 @@ class TopLevelMixin(mixins.ConfigMethodMixin):
         return copy
 
     def add_selection(self, *selections):
-        """Add one or more selections to the chart"""
+        """Add one or more selections to the chart."""
         if not selections:
             return self
-        else:
-            copy = self.copy(deep=True, ignore=['data'])
-            if copy.selection is Undefined:
-                copy.selection = SelectionMapping()
-            for selection in selections:
-                copy.selection += selection
-            return copy
+        copy = self.copy(deep=True, ignore=['data'])
+        if copy.selection is Undefined:
+            copy.selection = {}
+        for s in selections:
+            copy.selection[s.name] = s.selection
+        return copy
 
     def project(self, type='mercator', center=Undefined, clipAngle=Undefined, clipExtent=Undefined,
                 coefficient=Undefined, distance=Undefined, fraction=Undefined, lobes=Undefined,
@@ -961,7 +910,7 @@ class TopLevelMixin(mixins.ConfigMethodMixin):
             (2) a range predicate
             (3) a selection predicate
             (4) a logical operand combining (1)-(3)
-            (5) a NamedSelection object
+            (5) a Selection object
 
         Returns
         -------
@@ -975,8 +924,8 @@ class TopLevelMixin(mixins.ConfigMethodMixin):
         """
         selection_predicates = (core.SelectionNot, core.SelectionOr,
                                 core.SelectionAnd, core.SelectionOperand)
-        if isinstance(filter, NamedSelection):
-            filter = {'selection': filter._get_name()}
+        if isinstance(filter, Selection):
+            filter = {'selection': filter.name}
         elif isinstance(filter, selection_predicates):
             filter = {'selection': filter}
         return self._add_transform(core.FilterTransform(filter=filter, **kwargs))
@@ -1417,7 +1366,7 @@ class EncodingMixin(object):
                                      "".format(encoding))
                 kwargs[encoding] = arg
 
-        def _wrap_in_channel_class(obj, prop):
+        def _wrap_in_channel_class(obj, prop):  
             clsname = prop.title()
 
             if isinstance(obj, core.SchemaBase):
@@ -1598,15 +1547,8 @@ class Chart(TopLevelMixin, EncodingMixin, mixins.MarkMethodMixin,
             encodings.append('x')
         if bind_y:
             encodings.append('y')
-        copy = self.copy(deep=True, ignore=['data'])
-
-        if copy.selection is Undefined:
-            copy.selection = SelectionMapping()
-        if isinstance(copy.selection, dict):
-            copy.selection = SelectionMapping(**copy.selection)
-        copy.selection += selection(type='interval', bind='scales',
-                                    encodings=encodings)
-        return copy
+        return self.add_selection(selection_interval(bind='scales',
+                                                     encodings=encodings))
 
     def facet(self, row=Undefined, column=Undefined, data=Undefined, **kwargs):
         """Create a facet chart from the current chart.
diff --git a/doc/user_guide/compound_charts.rst b/doc/user_guide/compound_charts.rst
index 5829507b..e6251eab 100644
--- a/doc/user_guide/compound_charts.rst
+++ b/doc/user_guide/compound_charts.rst
@@ -181,7 +181,7 @@ with a ``brush`` selection to add interaction:
     brush = alt.selection(type='interval', encodings=['x'])
 
     upper = alt.Chart(sp500).mark_area().encode(
-        x=alt.X('date:T', scale={'domain': brush.ref()}),
+        x=alt.X('date:T', scale=alt.Scale(domain=brush),
         y='price:Q'
     ).properties(
         width=600,
diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py
index 0649e967..ab128ece 100644
--- a/tools/schemapi/schemapi.py
+++ b/tools/schemapi/schemapi.py
@@ -239,6 +239,8 @@ class SchemaBase(object):
             elif isinstance(val, dict):
                 return {k: _todict(v) for k, v in val.items()
                         if v is not Undefined}
+            elif hasattr(val, 'to_dict'):
+                return val.to_dict()
             else:
                 return val
 


Test patch:
diff --git a/altair/vegalite/v3/tests/test_api.py b/altair/vegalite/v3/tests/test_api.py
index 552536a5..49c803af 100644
--- a/altair/vegalite/v3/tests/test_api.py
+++ b/altair/vegalite/v3/tests/test_api.py
@@ -254,35 +254,29 @@ def test_facet_parse_data():
                             'row': {'field': 'row', 'type': 'nominal'}}
 
 
-def test_SelectionMapping():
+def test_selection():
     # test instantiation of selections
     interval = alt.selection_interval(name='selec_1')
-    assert interval['selec_1'].type == 'interval'
-    assert interval._get_name() == 'selec_1'
+    assert interval.selection.type == 'interval'
+    assert interval.name == 'selec_1'
 
     single = alt.selection_single(name='selec_2')
-    assert single['selec_2'].type == 'single'
-    assert single._get_name() == 'selec_2'
+    assert single.selection.type == 'single'
+    assert single.name == 'selec_2'
 
     multi = alt.selection_multi(name='selec_3')
-    assert multi['selec_3'].type == 'multi'
-    assert multi._get_name() == 'selec_3'
+    assert multi.selection.type == 'multi'
+    assert multi.name == 'selec_3'
 
-    # test addition
-    x = single + multi + interval
-    assert set(x.to_dict().keys()) == {'selec_1', 'selec_2', 'selec_3'}
-
-    y = single.copy()
-    y += multi
-    y += interval
-    assert x.to_dict() == y.to_dict()
+    # test adding to chart
+    chart = alt.Chart().add_selection(single)
+    chart = chart.add_selection(multi, interval)
+    assert set(chart.selection.keys()) == {'selec_1', 'selec_2', 'selec_3'}
 
     # test logical operations
-    x = single & multi
-    assert isinstance(x, alt.SelectionAnd)
-
-    y = single | multi
-    assert isinstance(y, alt.SelectionOr)
+    assert isinstance(single & multi, alt.SelectionAnd)
+    assert isinstance(single | multi, alt.SelectionOr)
+    assert isinstance(~single, alt.SelectionNot)
 
 
 def test_transforms():
@@ -416,8 +410,8 @@ def test_add_selection():
         selections[1],
         selections[2]
     )
-    expected = selections[0] + selections[1] + selections[2]
-    assert chart.selection.to_dict() == expected.to_dict()
+    expected = {s.name: s.selection for s in selections}
+    assert chart.selection == expected
 
 
 def test_LookupData():