From 6b9b9fb8e7df0f5849acfabd3dcbe1e16a5465e6 Mon Sep 17 00:00:00 2001 From: dmichalowicz Date: Wed, 15 Jun 2016 17:38:17 -0400 Subject: [PATCH 1/2] BUG: Fail fast on invalid pipeline columns --- tests/pipeline/test_pipeline.py | 6 ++++++ zipline/pipeline/pipeline.py | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/pipeline/test_pipeline.py b/tests/pipeline/test_pipeline.py index 74661fc6..9eb686d1 100644 --- a/tests/pipeline/test_pipeline.py +++ b/tests/pipeline/test_pipeline.py @@ -63,6 +63,9 @@ class PipelineTestCase(TestCase): with self.assertRaises(TypeError): Pipeline({}, SomeFactor()) + with self.assertRaises(TypeError): + Pipeline({'open': USEquityPricing.open}) + Pipeline({}, SomeFactor() > 5) def test_add(self): @@ -78,6 +81,9 @@ class PipelineTestCase(TestCase): with self.assertRaises(TypeError): p.add(f, 1) + with self.assertRaises(TypeError): + p.add(USEquityPricing.open, 'open') + def test_overwrite(self): p = Pipeline() f = SomeFactor() diff --git a/zipline/pipeline/pipeline.py b/zipline/pipeline/pipeline.py index 7bfaa4b1..ab40c886 100644 --- a/zipline/pipeline/pipeline.py +++ b/zipline/pipeline/pipeline.py @@ -1,6 +1,6 @@ from zipline.utils.input_validation import expect_types, optional -from .term import Term, AssetExists +from .term import AssetExists, ComputableTerm from .filters import Filter from .graph import TermGraph @@ -37,6 +37,12 @@ class Pipeline(object): if columns is None: columns = {} + for term in columns.values(): + if not isinstance(term, ComputableTerm): + raise TypeError( + '"{term}" is not a valid pipeline column. Did you mean ' + 'to add ".latest"?'.format(term=term) + ) self._columns = columns self._screen = screen @@ -54,7 +60,7 @@ class Pipeline(object): """ return self._screen - @expect_types(term=Term, name=str) + @expect_types(term=ComputableTerm, name=str) def add(self, term, name, overwrite=False): """ Add a column. From ed8947dfb34a6922e266740daff1edbb64b68059 Mon Sep 17 00:00:00 2001 From: dmichalowicz Date: Fri, 17 Jun 2016 18:18:46 -0400 Subject: [PATCH 2/2] Language tweaks --- zipline/pipeline/pipeline.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/zipline/pipeline/pipeline.py b/zipline/pipeline/pipeline.py index ab40c886..1967f727 100644 --- a/zipline/pipeline/pipeline.py +++ b/zipline/pipeline/pipeline.py @@ -1,6 +1,6 @@ from zipline.utils.input_validation import expect_types, optional -from .term import AssetExists, ComputableTerm +from .term import AssetExists, ComputableTerm, Term from .filters import Filter from .graph import TermGraph @@ -37,11 +37,13 @@ class Pipeline(object): if columns is None: columns = {} - for term in columns.values(): + for column_name, term in columns.items(): if not isinstance(term, ComputableTerm): raise TypeError( - '"{term}" is not a valid pipeline column. Did you mean ' - 'to add ".latest"?'.format(term=term) + "Column {column_name!r} contains an invalid pipeline term " + "({term}). Did you mean to append '.latest'?".format( + column_name=column_name, term=term, + ) ) self._columns = columns self._screen = screen @@ -60,7 +62,7 @@ class Pipeline(object): """ return self._screen - @expect_types(term=ComputableTerm, name=str) + @expect_types(term=Term, name=str) def add(self, term, name, overwrite=False): """ Add a column. @@ -85,6 +87,12 @@ class Pipeline(object): else: raise KeyError("Column '{}' already exists.".format(name)) + if not isinstance(term, ComputableTerm): + raise TypeError( + "{term} is not a valid pipeline column. Did you mean to " + "append '.latest'?".format(term=term) + ) + self._columns[name] = term @expect_types(name=str)