multi_order_trades does not do what it says on the tin
Opened this issue · 1 comments
This section also does not make sense to me. If self.multi_order_trades==True
and the trade is already live, the rest of the validations is skipped.
flumine/flumine/strategy/strategy.py
Lines 166 to 170 in 796ec51
Should this code not check if len(trade.orders) >= 1
and reject placement if self.multi_order_trades==False
? Allowing multiple orders per trade is very different from allowing multiple trades per runner, so the validation should proceed with the other checks. If a trade does not have at least two orders (opening and closing), I would not even call it a trade but a straight bet.
Originally posted by @mzaja in #679 (comment)
Current behaviour
multi_order_trades is True
: If a trade is already live allow it to be reused any number of times.multi_order_trades is False
: Validate the trade before order placement. It is possible to place any number of orders with a single trade as long astrade_count
andlive_trade_count
are not exceeded. A Trade object can still be reused to place multiple orders.multi_order_trades
's value is irrelevant unless the number of trades or live trades is at the limit or has been exceeded, in which case it acts likeforce=True
. Post bugfix in #682, this will change to "irrelevant unless the number of trades or live trades has been exceeded"
Expected behaviour
multi_order_trades is True
: Do nothing - proceeed with validation as usual.multi_order_trades is False
: Reject order iflen(trade.orders) >= 1
.
Proposal
Since the current implementation of multi_order_trades
argument does not do much on its own and certainly does not do what the name suggest, I propose replacing it with per-trade order limits: max_order_count
and max_live_order_count
. These would behave on the trade level the same as max_trade_count
and max_live_trade_count
behave on the strategy level. To maintain backwards compatibility, they should be None
by default and optionally set by the user.