diff --git a/freqtrade/resolvers/strategy_resolver.py b/freqtrade/resolvers/strategy_resolver.py index 7610b6fe8..8cdc5f614 100644 --- a/freqtrade/resolvers/strategy_resolver.py +++ b/freqtrade/resolvers/strategy_resolver.py @@ -222,21 +222,25 @@ class StrategyResolver(IResolver): if strategy: if strategy.config.get('trading_mode', TradingMode.SPOT) != TradingMode.SPOT: # Require new method - if check_override(strategy, IStrategy, 'populate_entry_trend'): + if not check_override(strategy, IStrategy, 'populate_entry_trend'): raise OperationalException("`populate_entry_trend` must be implemented.") - if check_override(strategy, IStrategy, 'populate_exit_trend'): + if not check_override(strategy, IStrategy, 'populate_exit_trend'): raise OperationalException("`populate_exit_trend` must be implemented.") + if check_override(strategy, IStrategy, 'custom_sell'): + raise OperationalException( + "Please migrate your implementation of `custom_sell` to `custom_exit`.") else: - # TODO: Implementing buy_trend and sell_trend should show a deprecation warning + # TODO: Implementing one of the following methods should show a deprecation warning + # buy_trend and sell_trend, custom_sell if ( - check_override(strategy, IStrategy, 'populate_buy_trend') - and check_override(strategy, IStrategy, 'populate_entry_trend') + not check_override(strategy, IStrategy, 'populate_buy_trend') + and not check_override(strategy, IStrategy, 'populate_entry_trend') ): raise OperationalException( "`populate_entry_trend` or `populate_buy_trend` must be implemented.") if ( - check_override(strategy, IStrategy, 'populate_sell_trend') - and check_override(strategy, IStrategy, 'populate_exit_trend') + not check_override(strategy, IStrategy, 'populate_sell_trend') + and not check_override(strategy, IStrategy, 'populate_exit_trend') ): raise OperationalException( "`populate_exit_trend` or `populate_sell_trend` must be implemented.") @@ -262,5 +266,6 @@ class StrategyResolver(IResolver): def check_override(object, parentclass, attribute): """ Checks if a object overrides the parent class attribute. + :returns: True if the object is overridden. """ - return getattr(type(object), attribute) == getattr(parentclass, attribute) + return getattr(type(object), attribute) != getattr(parentclass, attribute) diff --git a/freqtrade/strategy/interface.py b/freqtrade/strategy/interface.py index 93b017c60..975e9d41f 100644 --- a/freqtrade/strategy/interface.py +++ b/freqtrade/strategy/interface.py @@ -29,7 +29,7 @@ from freqtrade.wallets import Wallets logger = logging.getLogger(__name__) -CUSTOM_SELL_MAX_LENGTH = 64 +CUSTOM_EXIT_MAX_LENGTH = 64 class SellCheckTuple: @@ -380,6 +380,7 @@ class IStrategy(ABC, HyperStrategyMixin): def custom_sell(self, pair: str, trade: Trade, current_time: datetime, current_rate: float, current_profit: float, **kwargs) -> Optional[Union[str, bool]]: """ + DEPRECATED - please use custom_exit instead. Custom exit signal logic indicating that specified position should be sold. Returning a string or True from this method is equal to setting exit signal on a candle at specified time. This method is not called when exit signal is set. @@ -401,6 +402,30 @@ class IStrategy(ABC, HyperStrategyMixin): """ return None + def custom_exit(self, pair: str, trade: Trade, current_time: datetime, current_rate: float, + current_profit: float, **kwargs) -> Optional[Union[str, bool]]: + """ + Custom exit signal logic indicating that specified position should be sold. Returning a + string or True from this method is equal to setting exit signal on a candle at specified + time. This method is not called when exit signal is set. + + This method should be overridden to create exit signals that depend on trade parameters. For + example you could implement an exit relative to the candle when the trade was opened, + or a custom 1:2 risk-reward ROI. + + Custom exit reason max length is 64. Exceeding characters will be removed. + + :param pair: Pair that's currently analyzed + :param trade: trade object. + :param current_time: datetime object, containing the current datetime + :param current_rate: Rate, calculated based on pricing settings in ask_strategy. + :param current_profit: Current profit (as ratio), calculated based on current_rate. + :param **kwargs: Ensure to keep this here so updates to this won't break your strategy. + :return: To execute exit, return a string with custom sell reason or True. Otherwise return + None or False. + """ + return self.custom_sell(pair, trade, current_time, current_rate, current_profit, **kwargs) + def custom_stake_amount(self, pair: str, current_time: datetime, current_rate: float, proposed_stake: float, min_stake: float, max_stake: float, entry_tag: Optional[str], side: str, **kwargs) -> float: @@ -866,17 +891,17 @@ class IStrategy(ABC, HyperStrategyMixin): sell_signal = SellType.SELL_SIGNAL else: trade_type = "exit_short" if trade.is_short else "sell" - custom_reason = strategy_safe_wrapper(self.custom_sell, default_retval=False)( + custom_reason = strategy_safe_wrapper(self.custom_exit, default_retval=False)( pair=trade.pair, trade=trade, current_time=current_time, current_rate=current_rate, current_profit=current_profit) if custom_reason: sell_signal = SellType.CUSTOM_SELL if isinstance(custom_reason, str): - if len(custom_reason) > CUSTOM_SELL_MAX_LENGTH: + if len(custom_reason) > CUSTOM_EXIT_MAX_LENGTH: logger.warning(f'Custom {trade_type} reason returned from ' - f'custom_{trade_type} is too long and was trimmed' - f'to {CUSTOM_SELL_MAX_LENGTH} characters.') - custom_reason = custom_reason[:CUSTOM_SELL_MAX_LENGTH] + f'custom_exit is too long and was trimmed' + f'to {CUSTOM_EXIT_MAX_LENGTH} characters.') + custom_reason = custom_reason[:CUSTOM_EXIT_MAX_LENGTH] else: custom_reason = None if sell_signal in (SellType.CUSTOM_SELL, SellType.SELL_SIGNAL): diff --git a/freqtrade/templates/subtemplates/strategy_methods_advanced.j2 b/freqtrade/templates/subtemplates/strategy_methods_advanced.j2 index d0b56fe8e..d98adfa07 100644 --- a/freqtrade/templates/subtemplates/strategy_methods_advanced.j2 +++ b/freqtrade/templates/subtemplates/strategy_methods_advanced.j2 @@ -92,7 +92,7 @@ def custom_stoploss(self, pair: str, trade: 'Trade', current_time: 'datetime', """ return self.stoploss -def custom_sell(self, pair: str, trade: 'Trade', current_time: 'datetime', current_rate: float, +def custom_exit(self, pair: str, trade: 'Trade', current_time: 'datetime', current_rate: float, current_profit: float, **kwargs) -> 'Optional[Union[str, bool]]': """ Custom sell signal logic indicating that specified position should be sold. Returning a diff --git a/tests/strategy/strats/broken_strats/broken_futures_strategies.py b/tests/strategy/strats/broken_strats/broken_futures_strategies.py index 42f631b97..4a84b7491 100644 --- a/tests/strategy/strats/broken_strats/broken_futures_strategies.py +++ b/tests/strategy/strats/broken_strats/broken_futures_strategies.py @@ -1,4 +1,9 @@ -# The strategy which fails to load due to non-existent dependency +""" +The strategies here are minimal strategies designed to fail loading in certain conditions. +They are not operational, and don't aim to be. +""" + +from datetime import datetime from pandas import DataFrame @@ -14,3 +19,13 @@ class TestStrategyNoImplements(IStrategy): class TestStrategyNoImplementSell(TestStrategyNoImplements): def populate_entry_trend(self, dataframe: DataFrame, metadata: dict) -> DataFrame: return super().populate_entry_trend(dataframe, metadata) + + +class TestStrategyImplementCustomSell(TestStrategyNoImplementSell): + def populate_exit_trend(self, dataframe: DataFrame, metadata: dict) -> DataFrame: + return super().populate_exit_trend(dataframe, metadata) + + def custom_sell(self, pair: str, trade, current_time: datetime, + current_rate: float, current_profit: float, + **kwargs): + return False diff --git a/tests/strategy/test_interface.py b/tests/strategy/test_interface.py index 4a01b7dec..18af215a3 100644 --- a/tests/strategy/test_interface.py +++ b/tests/strategy/test_interface.py @@ -477,7 +477,7 @@ def test_stop_loss_reached(default_conf, fee, profit, adjusted, expected, traili strategy.custom_stoploss = original_stopvalue -def test_custom_sell(default_conf, fee, caplog) -> None: +def test_custom_exit(default_conf, fee, caplog) -> None: strategy = StrategyResolver.load_strategy(default_conf) trade = Trade( @@ -499,7 +499,7 @@ def test_custom_sell(default_conf, fee, caplog) -> None: assert res.sell_flag is False assert res.sell_type == SellType.NONE - strategy.custom_sell = MagicMock(return_value=True) + strategy.custom_exit = MagicMock(return_value=True) res = strategy.should_exit(trade, 1, now, enter=False, exit_=False, low=None, high=None) @@ -507,7 +507,7 @@ def test_custom_sell(default_conf, fee, caplog) -> None: assert res.sell_type == SellType.CUSTOM_SELL assert res.sell_reason == 'custom_sell' - strategy.custom_sell = MagicMock(return_value='hello world') + strategy.custom_exit = MagicMock(return_value='hello world') res = strategy.should_exit(trade, 1, now, enter=False, exit_=False, @@ -517,14 +517,14 @@ def test_custom_sell(default_conf, fee, caplog) -> None: assert res.sell_reason == 'hello world' caplog.clear() - strategy.custom_sell = MagicMock(return_value='h' * 100) + strategy.custom_exit = MagicMock(return_value='h' * 100) res = strategy.should_exit(trade, 1, now, enter=False, exit_=False, low=None, high=None) assert res.sell_type == SellType.CUSTOM_SELL assert res.sell_flag is True assert res.sell_reason == 'h' * 64 - assert log_has_re('Custom sell reason returned from custom_sell is too long.*', caplog) + assert log_has_re('Custom sell reason returned from custom_exit is too long.*', caplog) @pytest.mark.parametrize('side', TRADE_SIDES) diff --git a/tests/strategy/test_strategy_loading.py b/tests/strategy/test_strategy_loading.py index 7e0c796f4..7464c3330 100644 --- a/tests/strategy/test_strategy_loading.py +++ b/tests/strategy/test_strategy_loading.py @@ -392,7 +392,7 @@ def test_deprecate_populate_indicators(result, default_conf): @pytest.mark.filterwarnings("ignore:deprecated") -def test_missing_implements(result, default_conf): +def test_missing_implements(default_conf): default_location = Path(__file__).parent / "strats/broken_strats" default_conf.update({'strategy': 'TestStrategyNoImplements', 'strategy_path': default_location}) @@ -406,6 +406,7 @@ def test_missing_implements(result, default_conf): match=r"`populate_exit_trend` or `populate_sell_trend`.*"): StrategyResolver.load_strategy(default_conf) + # Futures mode is more strict ... default_conf['trading_mode'] = 'futures' with pytest.raises(OperationalException, @@ -417,6 +418,12 @@ def test_missing_implements(result, default_conf): match=r"`populate_entry_trend` must be implemented.*"): StrategyResolver.load_strategy(default_conf) + default_conf['strategy'] = 'TestStrategyImplementCustomSell' + + with pytest.raises(OperationalException, + match=r"Please migrate your implementation of `custom_sell`.*"): + StrategyResolver.load_strategy(default_conf) + @pytest.mark.filterwarnings("ignore:deprecated") def test_call_deprecated_function(result, default_conf, caplog):