Repository: altair-viz/altair

Issue:
repeat() function is borked in Altair 3
Need to make certain ``alt.repeat()`` and ``alt.Chart.repeat()`` work correctly together and support wrapped repeats.

Discussion:

Patch:
diff --git a/altair/vegalite/v3/api.py b/altair/vegalite/v3/api.py
index 16796458..a57e009c 100644
--- a/altair/vegalite/v3/api.py
+++ b/altair/vegalite/v3/api.py
@@ -487,7 +487,7 @@ class TopLevelMixin(mixins.ConfigMethodMixin):
     def __or__(self, other):
         return HConcatChart(hconcat=[self, other])
 
-    def repeat(self, row=Undefined, column=Undefined, **kwargs):
+    def repeat(self, repeat=Undefined, row=Undefined, column=Undefined, columns=Undefined, **kwargs):
         """Return a RepeatChart built from the chart
 
         Fields within the chart can be set to correspond to the row or
@@ -495,19 +495,36 @@ class TopLevelMixin(mixins.ConfigMethodMixin):
 
         Parameters
         ----------
+        repeat : list
+            a list of data column names to be repeated. This cannot be
+            used along with the ``row`` or ``column`` argument.
         row : list
             a list of data column names to be mapped to the row facet
         column : list
             a list of data column names to be mapped to the column facet
+        columns : int
+            the maximum number of columns before wrapping. Only referenced
+            if ``repeat`` is specified.
+        **kwargs :
+            additional keywords passed to RepeatChart.
 
         Returns
         -------
         chart : RepeatChart
             a repeated chart.
-
         """
-        repeat = core.RepeatMapping(row=row, column=column)
-        return RepeatChart(spec=self, repeat=repeat, **kwargs)
+        repeat_specified = (repeat is not Undefined)
+        rowcol_specified = (row is not Undefined or column is not Undefined)
+
+        if repeat_specified and rowcol_specified:
+            raise ValueError("repeat argument cannot be combined with row/column argument.")
+
+        if repeat_specified:
+            repeat = repeat
+        else:
+            repeat = core.RepeatMapping(row=row, column=column)
+
+        return RepeatChart(spec=self, repeat=repeat, columns=columns, **kwargs)
 
     def properties(self, **kwargs):
         """Set top-level properties of the Chart.
@@ -1378,7 +1395,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):
@@ -1662,7 +1679,7 @@ class RepeatChart(TopLevelMixin, core.TopLevelRepeatSpec):
         return copy
 
 
-def repeat(repeater):
+def repeat(repeater='repeat'):
     """Tie a channel to the row or column within a repeated chart
 
     The output of this should be passed to the ``field`` attribute of
@@ -1671,13 +1688,14 @@ def repeat(repeater):
     Parameters
     ----------
     repeater : {'row'|'column'|'repeat'}
-        The repeater to tie the field to.
+        The repeater to tie the field to. Default is 'repeat'.
 
     Returns
     -------
     repeat : RepeatRef object
     """
-    assert repeater in ['row', 'column', 'repeat']
+    if repeater not in ['row', 'column', 'repeat']:
+        raise ValueError("repeater must be one of ['row', 'column', 'repeat']")
     return core.RepeatRef(repeat=repeater)
 
 


Test patch:
diff --git a/altair/vegalite/v3/tests/test_api.py b/altair/vegalite/v3/tests/test_api.py
index e9f9143d..119ed6eb 100644
--- a/altair/vegalite/v3/tests/test_api.py
+++ b/altair/vegalite/v3/tests/test_api.py
@@ -493,9 +493,10 @@ def test_chart_from_dict():
               base & base,
               base.facet(row='c:N'),
               (base + base).facet(row='c:N', data='data.csv'),
-              base.repeat(row=['c:N', 'd:N'])]
+              base.repeat(['c:N', 'd:N'])]
 
     for chart in charts:
+        print(chart)
         chart_out = alt.Chart.from_dict(chart.to_dict())
         assert type(chart_out) is type(chart)
 
@@ -571,4 +572,37 @@ def test_deprecated_encodings():
     assert 'alt.StrokeOpacity' in record[0].message.args[0]
     chart2 = base.encode(strokeOpacity=alt.StrokeOpacity('x:Q')).to_dict()
 
-    assert chart1 == chart2
\ No newline at end of file
+    assert chart1 == chart2
+
+
+def test_repeat():
+    # wrapped repeat
+    chart1 = alt.Chart('data.csv').mark_point().encode(
+        x=alt.X(alt.repeat(), type='quantitative'),
+        y='y:Q',
+    ).repeat(
+        ['A', 'B', 'C', 'D'],
+        columns=2
+    )
+
+    dct1 = chart1.to_dict()
+
+    assert dct1['repeat'] == ['A', 'B', 'C', 'D']
+    assert dct1['columns'] == 2
+    assert dct1['spec']['encoding']['x']['field'] == {'repeat': 'repeat'}
+
+    # explicit row/col repeat
+    chart2 = alt.Chart('data.csv').mark_point().encode(
+        x=alt.X(alt.repeat('row'), type='quantitative'),
+        y=alt.Y(alt.repeat('column'), type='quantitative')
+    ).repeat(
+        row=['A', 'B', 'C'],
+        column=['C', 'B', 'A']
+    )
+
+    dct2 = chart2.to_dict()
+
+    assert dct2['repeat'] == {'row': ['A', 'B', 'C'], 'column': ['C', 'B', 'A']}
+    assert 'columns' not in dct2
+    assert dct2['spec']['encoding']['x']['field'] == {'repeat': 'row'}
+    assert dct2['spec']['encoding']['y']['field'] == {'repeat': 'column'}