BUG: Fix simulation correctness bugs in slippage, cancel policy, and order params

docs/source/whatsnew/skeleton.txt

@@ -31,7 +31,21 @@ None
 Bug Fixes
 ~~~~~~~~~
 
-None
+- Fix ``execute_daily_cancel_policy`` to cancel all orders for all assets at
+  end of day, not just the first order for assets with multiple orders.
+  Previously, assets with a single open order were skipped entirely. (:issue:`313`)
+- Fix ``NoSlippage.process_order`` and ``FixedSlippage.process_order`` to use
+  ``order.open_amount`` instead of ``order.amount``, preventing overfill of
+  partially filled orders. (:issue:`313`)
+- Fix ``VolumeShareSlippage.process_order`` to return ``(None, None)`` instead
+  of bare ``None`` when price is NaN, preventing ``TypeError`` during tuple
+  unpacking. (:issue:`313`)
+- Fix ``before_trading_start`` to reset ``_in_before_trading_start`` flag via
+  ``try/finally``, preventing orders from being permanently blocked after an
+  exception in the user's callback. (:issue:`313`)
+- Fix ``validate_order_params`` and ``__convert_order_params_for_blotter`` to
+  use ``is not None`` checks for ``limit_price`` and ``stop_price`` instead of
+  truthiness checks, allowing a price of ``0.0``. (:issue:`313`)
 
 Performance
 ~~~~~~~~~~~

src/zipline/algorithm.py

@@ -431,14 +431,15 @@ def before_trading_start(self, data):
 
         self._in_before_trading_start = True
 
-        with (
-            handle_non_market_minutes(data)
-            if self.data_frequency == "minute"
-            else ExitStack()
-        ):
-            self._before_trading_start(self, data)
-
-        self._in_before_trading_start = False
+        try:
+            with (
+                handle_non_market_minutes(data)
+                if self.data_frequency == "minute"
+                else ExitStack()
+            ):
+                self._before_trading_start(self, data)
+        finally:
+            self._in_before_trading_start = False
 
     def handle_data(self, data):
         if self._handle_data:
@@ -1301,12 +1302,12 @@ def validate_order_params(self, asset, amount, limit_price, stop_price, style):
             )
 
         if style:
-            if limit_price:
+            if limit_price is not None:
                 raise UnsupportedOrderParameters(
                     msg="Passing both limit_price and style is not supported."
                 )
 
-            if stop_price:
+            if stop_price is not None:
                 raise UnsupportedOrderParameters(
                     msg="Passing both stop_price and style is not supported."
                 )
@@ -1331,11 +1332,11 @@ def __convert_order_params_for_blotter(asset, limit_price, stop_price, style):
         if style:
             assert (limit_price, stop_price) == (None, None)
             return style
-        if limit_price and stop_price:
+        if limit_price is not None and stop_price is not None:
             return StopLimitOrder(limit_price, stop_price, asset=asset)
-        if limit_price:
+        if limit_price is not None:
             return LimitOrder(limit_price, asset=asset)
-        if stop_price:
+        if stop_price is not None:
             return StopOrder(stop_price, asset=asset)
         else:
             return MarketOrder()

src/zipline/finance/blotter/simulation_blotter.py

@@ -239,48 +239,7 @@ def execute_daily_cancel_policy(self, event):
         if self.cancel_policy.should_cancel(event):
             warn = self.cancel_policy.warn_on_cancel
             for asset in copy(self.open_orders):
-                orders = self.open_orders[asset]
-                if len(orders) > 1:
-                    order = orders[0]
-                    self.cancel(order.id, relay_status=True)
-                    if warn:
-                        if order.filled > 0:
-                            warning_logger.warn(
-                                "Your order for {order_amt} shares of "
-                                "{order_sym} has been partially filled. "
-                                "{order_filled} shares were successfully "
-                                "purchased. {order_failed} shares were not "
-                                "filled by the end of day and "
-                                "were canceled.".format(
-                                    order_amt=order.amount,
-                                    order_sym=order.asset.symbol,
-                                    order_filled=order.filled,
-                                    order_failed=order.amount - order.filled,
-                                )
-                            )
-                        elif order.filled < 0:
-                            warning_logger.warn(
-                                "Your order for {order_amt} shares of "
-                                "{order_sym} has been partially filled. "
-                                "{order_filled} shares were successfully "
-                                "sold. {order_failed} shares were not "
-                                "filled by the end of day and "
-                                "were canceled.".format(
-                                    order_amt=order.amount,
-                                    order_sym=order.asset.symbol,
-                                    order_filled=-1 * order.filled,
-                                    order_failed=-1 * (order.amount - order.filled),
-                                )
-                            )
-                        else:
-                            warning_logger.warn(
-                                "Your order for {order_amt} shares of "
-                                "{order_sym} failed to fill by the end of day "
-                                "and was canceled.".format(
-                                    order_amt=order.amount,
-                                    order_sym=order.asset.symbol,
-                                )
-                            )
+                self.cancel_all_orders_for_asset(asset, warn, relay_status=True)
 
     def execute_cancel_policy(self, event):
         if self.cancel_policy.should_cancel(event):

src/zipline/finance/slippage.py

@@ -222,7 +222,7 @@ class NoSlippage(SlippageModel):
     def process_order(data, order):
         return (
             data.current(order.asset, "close"),
-            order.amount,
+            order.open_amount,
         )
 
 
@@ -317,7 +317,7 @@ def process_order(self, data, order):
         # Remove this block after fixing data to ensure volume always has
         # corresponding price.
         if isnull(price):
-            return
+            return None, None
         # END
 
         simulated_impact = (
@@ -362,7 +362,7 @@ def __repr__(self):
     def process_order(self, data, order):
         price = data.current(order.asset, "close")
 
-        return (price + (self.spread / 2.0 * order.direction), order.amount)
+        return (price + (self.spread / 2.0 * order.direction), order.open_amount)
 
 
 class MarketImpactBase(SlippageModel):

tests/test_algorithm.py

@@ -3852,19 +3852,19 @@ def test_default_cancelation_policy(self):
             assert len(self._caplog.messages) == 0
 
     def test_eod_order_cancel_daily(self):
-        # in daily mode, EODCancel does nothing.
+        # EODCancel cancels unfilled orders at end of day in daily mode.
         algo = self.prep_algo("set_cancel_policy(cancel_policy.EODCancel())", "daily")
 
         results = algo.run()
 
-        # order stays open throughout simulation
-        np.testing.assert_array_equal([1, 1, 1], list(map(len, results.orders)))
+        # order placed on day 0, cancelled at EOD, relay reported on day 1
+        np.testing.assert_array_equal([1, 1, 0], list(map(len, results.orders)))
 
-        # one txn per day
-        np.testing.assert_array_equal([0, 1, 1], list(map(len, results.transactions)))
+        # no transactions since order is cancelled before any fill
+        np.testing.assert_array_equal([0, 0, 0], list(map(len, results.transactions)))
 
         with self._caplog.at_level(logging.WARNING):
-            assert len(self._caplog.messages) == 0
+            assert len(self._caplog.messages) == 1
 
 
 class TestDailyEquityAutoClose(zf.WithMakeAlgo, zf.ZiplineTestCase):