From 4d42cddae42a3ec75a8e7cf57b3f3579c496c2c4 Mon Sep 17 00:00:00 2001
From: Scott Sanderson <ssanderson@quantopian.com>
Date: Wed, 4 May 2016 14:27:57 -0400
Subject: [PATCH] ENH: Fail fast on outputs in CustomClassifier.

We don't support multiple outputs for CustomClassifier because we use
LabelArrays for string classifiers.
---
 tests/pipeline/test_term.py                | 33 +++++++++++++++++++
 zipline/pipeline/classifiers/classifier.py | 37 ++++++++++++++--------
 zipline/pipeline/mixins.py                 | 16 ++++++++++
 3 files changed, 72 insertions(+), 14 deletions(-)

diff --git a/tests/pipeline/test_term.py b/tests/pipeline/test_term.py
index 408cb689..502d6e48 100644
--- a/tests/pipeline/test_term.py
+++ b/tests/pipeline/test_term.py
@@ -16,6 +16,7 @@ from zipline.errors import (
 )
 from zipline.pipeline import (
     Classifier,
+    CustomClassifier,
     CustomFactor,
     Factor,
     Filter,
@@ -25,8 +26,10 @@ from zipline.pipeline.data import Column, DataSet
 from zipline.pipeline.data.testing import TestingDataSet
 from zipline.pipeline.term import AssetExists, NotSpecified
 from zipline.pipeline.expression import NUMEXPR_MATH_FUNCS
+from zipline.testing import parameter_space
 from zipline.utils.numpy_utils import (
     bool_dtype,
+    categorical_dtype,
     complex128_dtype,
     datetime64ns_dtype,
     float64_dtype,
@@ -569,3 +572,33 @@ class SubDataSetTestCase(TestCase):
                 'subclass column %r should have the same dtype as the parent' %
                 k,
             )
+
+    @parameter_space(
+        dtype_=[categorical_dtype, int64_dtype],
+        outputs_=[('a',), ('a', 'b')],
+    )
+    def test_reject_multi_output_classifiers(self, dtype_, outputs_):
+        """
+        Multi-output CustomClassifiers don't work because they use special
+        output allocation for string arrays.
+        """
+
+        class SomeClassifier(CustomClassifier):
+            dtype = dtype_
+            window_length = 5
+            inputs = [SomeDataSet.foo, SomeDataSet.bar]
+            outputs = outputs_
+            missing_value = dtype_.type('123')
+
+        expected_error = (
+            "SomeClassifier does not support custom outputs, "
+            "but received custom outputs={outputs}.".format(outputs=outputs_)
+        )
+
+        with self.assertRaises(ValueError) as e:
+            SomeClassifier()
+        self.assertEqual(str(e.exception), expected_error)
+
+        with self.assertRaises(ValueError) as e:
+            SomeClassifier()
+        self.assertEqual(str(e.exception), expected_error)
diff --git a/zipline/pipeline/classifiers/classifier.py b/zipline/pipeline/classifiers/classifier.py
index d35a5135..5b33dba3 100644
--- a/zipline/pipeline/classifiers/classifier.py
+++ b/zipline/pipeline/classifiers/classifier.py
@@ -25,6 +25,7 @@ from ..mixins import (
     PositiveWindowLengthMixin,
     RestrictedDTypeMixin,
     SingleInputMixin,
+    StandardOutputs,
 )
 
 
@@ -351,16 +352,35 @@ class StringPredicate(SingleInputMixin, Filter):
         )
 
 
-class CustomClassifier(PositiveWindowLengthMixin, CustomTermMixin, Classifier):
+class CustomClassifier(PositiveWindowLengthMixin,
+                       StandardOutputs,
+                       CustomTermMixin,
+                       Classifier):
     """
     Base class for user-defined Classifiers.
 
+    Does not suppport multiple outputs.
+
     See Also
     --------
     zipline.pipeline.CustomFactor
     zipline.pipeline.CustomFilter
     """
-    pass
+    def _allocate_output(self, windows, shape):
+        """
+        Override the default array allocation to produce a LabelArray when we
+        have a string-like dtype.
+        """
+        if self.dtype == int64_dtype:
+            return super(CustomClassifier, self)._allocate_output(
+                windows,
+                shape,
+            )
+
+        # This is a little bit of a hack.  We might not know what the
+        # categories for a LabelArray are until it's actually been loaded, so
+        # we need to look at the underlying data.
+        return windows[0].data.empty_like(shape)
 
 
 class Latest(LatestMixin, CustomClassifier):
@@ -373,18 +393,7 @@ class Latest(LatestMixin, CustomClassifier):
     zipline.pipeline.factors.factor.Latest
     zipline.pipeline.filters.filter.Latest
     """
-    def _allocate_output(self, windows, shape):
-        """
-        Override the default array allocation to produce a LabelArray when we
-        have a string-like dtype.
-        """
-        if self.dtype == int64_dtype:
-            return super(Latest, self)._allocate_output(windows, shape)
-
-        # This is a little bit of a hack.  We might not know what the
-        # categories for a LabelArray are until it's actually been loaded, so
-        # we need to look at the underlying data.
-        return windows[0].data.empty_like(shape)
+    pass
 
 
 class InvalidClassifierComparison(TypeError):
diff --git a/zipline/pipeline/mixins.py b/zipline/pipeline/mixins.py
index 497fc145..e30b4b57 100644
--- a/zipline/pipeline/mixins.py
+++ b/zipline/pipeline/mixins.py
@@ -36,6 +36,22 @@ class SingleInputMixin(object):
             )
 
 
+class StandardOutputs(object):
+    """
+    Validation mixin enforcing that a Term cannot produce non-standard outputs.
+    """
+    def _validate(self):
+        super(StandardOutputs, self)._validate()
+        if self.outputs is not NotSpecified:
+            raise ValueError(
+                "{typename} does not support custom outputs,"
+                " but received custom outputs={outputs}.".format(
+                    typename=type(self).__name__,
+                    outputs=self.outputs,
+                )
+            )
+
+
 class RestrictedDTypeMixin(object):
     """
     Validation mixin enforcing that a term has a specific dtype.