overload populate_indicators to work with and without pair argumen

all while not breaking users strategies

freqtrade/strategy/default_strategy.py

@@ -28,7 +28,7 @@ class DefaultStrategy(IStrategy):
     # Optimal ticker interval for the strategy
     ticker_interval = '5m'
 
-    def advise_indicators(self, dataframe: DataFrame, pair: str) -> DataFrame:
+    def populate_indicators(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Adds several different TA indicators to the given DataFrame
 
@@ -196,7 +196,7 @@ class DefaultStrategy(IStrategy):
 
         return dataframe
 
-    def advise_buy(self, dataframe: DataFrame, pair: str) -> DataFrame:
+    def populate_buy_trend(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Based on TA indicators, populates the buy signal for the given dataframe
         :param dataframe: DataFrame
@@ -218,7 +218,7 @@ class DefaultStrategy(IStrategy):
 
         return dataframe
 
-    def advise_sell(self, dataframe: DataFrame, pair: str) -> DataFrame:
+    def populate_sell_trend(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Based on TA indicators, populates the sell signal for the given dataframe
         :param dataframe: DataFrame

freqtrade/strategy/interface.py

@@ -3,7 +3,7 @@ IStrategy interface
 This module defines the interface to apply for strategies
 """
 import logging
-from abc import ABC
+from abc import ABC, abstractmethod
 from datetime import datetime
 from enum import Enum
 from typing import Dict, List, NamedTuple, Tuple
@@ -70,37 +70,32 @@ class IStrategy(ABC):
     def __init__(self, config: dict) -> None:
         self.config = config
 
-    def populate_indicators(self, dataframe: DataFrame) -> DataFrame:
+    @abstractmethod
+    def populate_indicators(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Populate indicators that will be used in the Buy and Sell strategy
         :param dataframe: Raw data from the exchange and parsed by parse_ticker_dataframe()
+        :param pair: Pair currently analyzed
         :return: a Dataframe with all mandatory indicators for the strategies
         """
-        warnings.warn("deprecated - please replace this method with advise_indicators!",
-                      DeprecationWarning)
-        return dataframe
 
-    def populate_buy_trend(self, dataframe: DataFrame) -> DataFrame:
+    @abstractmethod
+    def populate_buy_trend(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Based on TA indicators, populates the buy signal for the given dataframe
         :param dataframe: DataFrame
+        :param pair: Pair currently analyzed
         :return: DataFrame with buy column
         """
-        warnings.warn("deprecated - please replace this method with advise_buy!",
-                      DeprecationWarning)
-        dataframe.loc[(), 'buy'] = 0
-        return dataframe
 
-    def populate_sell_trend(self, dataframe: DataFrame) -> DataFrame:
+    @abstractmethod
+    def populate_sell_trend(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Based on TA indicators, populates the sell signal for the given dataframe
         :param dataframe: DataFrame
+        :param pair: Pair currently analyzed
         :return: DataFrame with sell column
         """
-        warnings.warn("deprecated - please replace this method with advise_sell!",
-                      DeprecationWarning)
-        dataframe.loc[(), 'sell'] = 0
-        return dataframe
 
     def get_strategy_name(self) -> str:
         """
@@ -283,30 +278,44 @@ class IStrategy(ABC):
     def advise_indicators(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Populate indicators that will be used in the Buy and Sell strategy
-        If not overridden, calls the legacy method `populate_indicators to keep strategies working
+        This method should not be overridden.
         :param dataframe: Raw data from the exchange and parsed by parse_ticker_dataframe()
         :param pair: The currently traded pair
         :return: a Dataframe with all mandatory indicators for the strategies
         """
-        return self.populate_indicators(dataframe)
+        if len(self.populate_indicators.__annotations__) == 2:
+            warnings.warn("deprecated - check out the Sample strategy to see "
+                          "the current function headers!", DeprecationWarning)
+            return self.populate_indicators(dataframe)  # type: ignore
+        else:
+            return self.populate_indicators(dataframe, pair)
 
     def advise_buy(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Based on TA indicators, populates the buy signal for the given dataframe
-        If not overridden, calls the legacy method `populate_buy_trend to keep strategies working
+        This method should not be overridden.
         :param dataframe: DataFrame
         :param pair: The currently traded pair
         :return: DataFrame with buy column
         """
-
+        if len(self.populate_buy_trend.__annotations__) == 2:
-        return self.populate_buy_trend(dataframe)
+            warnings.warn("deprecated - check out the Sample strategy to see "
+                          "the current function headers!", DeprecationWarning)
+            return self.populate_buy_trend(dataframe)  # type: ignore
+        else:
+            return self.populate_buy_trend(dataframe, pair)
 
     def advise_sell(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Based on TA indicators, populates the sell signal for the given dataframe
-        If not overridden, calls the legacy method `populate_sell_trend to keep strategies working
+        This method should not be overridden.
         :param dataframe: DataFrame
         :param pair: The currently traded pair
         :return: DataFrame with sell column
         """
-        return self.populate_sell_trend(dataframe)
+        if len(self.populate_sell_trend.__annotations__) == 2:
+            warnings.warn("deprecated - check out the Sample strategy to see "
+                          "the current function headers!", DeprecationWarning)
+            return self.populate_sell_trend(dataframe)  # type: ignore
+        else:
+            return self.populate_sell_trend(dataframe, pair)

freqtrade/tests/strategy/test_default_strategy.py

@@ -25,10 +25,11 @@ def test_default_strategy_structure():
 def test_default_strategy(result):
     strategy = DefaultStrategy({})
 
+    pair = 'ETH/BTC'
     assert type(strategy.minimal_roi) is dict
     assert type(strategy.stoploss) is float
     assert type(strategy.ticker_interval) is str
-    indicators = strategy.populate_indicators(result)
+    indicators = strategy.populate_indicators(result, pair)
     assert type(indicators) is DataFrame
-    assert type(strategy.populate_buy_trend(indicators)) is DataFrame
+    assert type(strategy.populate_buy_trend(indicators, pair)) is DataFrame
-    assert type(strategy.populate_sell_trend(indicators)) is DataFrame
+    assert type(strategy.populate_sell_trend(indicators, pair)) is DataFrame

freqtrade/tests/strategy/test_strategy.py

@@ -1,10 +1,10 @@
 # pragma pylint: disable=missing-docstring, protected-access, C0103
 import logging
 from os import path
-from unittest.mock import MagicMock
 import warnings
 
 import pytest
+from pandas import DataFrame
 
 from freqtrade.strategy import import_strategy
 from freqtrade.strategy.default_strategy import DefaultStrategy
@@ -60,6 +60,9 @@ def test_search_strategy():
 def test_load_strategy(result):
     resolver = StrategyResolver({'strategy': 'TestStrategy'})
     pair = 'ETH/BTC'
+    assert len(resolver.strategy.populate_indicators.__annotations__) == 3
+    assert 'dataframe' in resolver.strategy.populate_indicators.__annotations__
+    assert 'pair' in resolver.strategy.populate_indicators.__annotations__
     assert 'adx' in resolver.strategy.advise_indicators(result, pair=pair)
 
 
@@ -158,39 +161,35 @@ def test_strategy_override_ticker_interval(caplog):
 
 
 def test_deprecate_populate_indicators(result):
-    resolver = StrategyResolver({'strategy': 'TestStrategy'})
+    default_location = path.join(path.dirname(path.realpath(__file__)))
+    resolver = StrategyResolver({'strategy': 'TestStrategyLegacy',
+                                 'strategy_path': default_location})
     with warnings.catch_warnings(record=True) as w:
         # Cause all warnings to always be triggered.
         warnings.simplefilter("always")
-        resolver.strategy.populate_indicators(result)
+        indicators = resolver.strategy.advise_indicators(result, 'ETH/BTC')
         assert len(w) == 1
         assert issubclass(w[-1].category, DeprecationWarning)
-        assert "deprecated - please replace this method with advise_indicators!" in str(
+        assert "deprecated - check out the Sample strategy to see the current function headers!" \
-            w[-1].message)
+            in str(w[-1].message)
 
+    with warnings.catch_warnings(record=True) as w:
+            # Cause all warnings to always be triggered.
+        warnings.simplefilter("always")
+        resolver.strategy.advise_buy(indicators, 'ETH/BTC')
+        assert len(w) == 1
+        assert issubclass(w[-1].category, DeprecationWarning)
+        assert "deprecated - check out the Sample strategy to see the current function headers!" \
+            in str(w[-1].message)
 
-def test_deprecate_populate_buy_trend(result):
-    resolver = StrategyResolver({'strategy': 'TestStrategy'})
     with warnings.catch_warnings(record=True) as w:
         # Cause all warnings to always be triggered.
         warnings.simplefilter("always")
-        resolver.strategy.populate_buy_trend(result)
+        resolver.strategy.advise_sell(indicators, 'ETH_BTC')
         assert len(w) == 1
         assert issubclass(w[-1].category, DeprecationWarning)
-        assert "deprecated - please replace this method with advise_buy!" in str(
+        assert "deprecated - check out the Sample strategy to see the current function headers!" \
-            w[-1].message)
+            in str(w[-1].message)
-
-
-def test_deprecate_populate_sell_trend(result):
-    resolver = StrategyResolver({'strategy': 'TestStrategy'})
-    with warnings.catch_warnings(record=True) as w:
-        # Cause all warnings to always be triggered.
-        warnings.simplefilter("always")
-        resolver.strategy.populate_sell_trend(result)
-        assert len(w) == 1
-        assert issubclass(w[-1].category, DeprecationWarning)
-        assert "deprecated - please replace this method with advise_sell!" in str(
-            w[-1].message)
 
 
 def test_call_deprecated_function(result, monkeypatch):
@@ -198,18 +197,26 @@ def test_call_deprecated_function(result, monkeypatch):
     resolver = StrategyResolver({'strategy': 'TestStrategyLegacy',
                                  'strategy_path': default_location})
     pair = 'ETH/BTC'
-    indicators_mock = MagicMock()
-    buy_trend_mock = MagicMock()
-    sell_trend_mock = MagicMock()
 
-    monkeypatch.setattr(resolver.strategy, 'populate_indicators', indicators_mock)
+    # Make sure we are using a legacy function
-    resolver.strategy.advise_indicators(result, pair=pair)
+    assert len(resolver.strategy.populate_indicators.__annotations__) == 2
-    assert indicators_mock.call_count == 1
+    assert 'dataframe' in resolver.strategy.populate_indicators.__annotations__
+    assert 'pair' not in resolver.strategy.populate_indicators.__annotations__
+    assert len(resolver.strategy.populate_buy_trend.__annotations__) == 2
+    assert 'dataframe' in resolver.strategy.populate_buy_trend.__annotations__
+    assert 'pair' not in resolver.strategy.populate_buy_trend.__annotations__
+    assert len(resolver.strategy.populate_sell_trend.__annotations__) == 2
+    assert 'dataframe' in resolver.strategy.populate_sell_trend.__annotations__
+    assert 'pair' not in resolver.strategy.populate_sell_trend.__annotations__
 
-    monkeypatch.setattr(resolver.strategy, 'populate_buy_trend', buy_trend_mock)
+    indicator_df = resolver.strategy.advise_indicators(result, pair=pair)
-    resolver.strategy.advise_buy(result, pair=pair)
+    assert type(indicator_df) is DataFrame
-    assert buy_trend_mock.call_count == 1
+    assert 'adx' in indicator_df.columns
 
-    monkeypatch.setattr(resolver.strategy, 'populate_sell_trend', sell_trend_mock)
+    buydf = resolver.strategy.advise_buy(result, pair=pair)
-    resolver.strategy.advise_sell(result, pair=pair)
+    assert type(buydf) is DataFrame
-    assert sell_trend_mock.call_count == 1
+    assert 'buy' in buydf.columns
+
+    selldf = resolver.strategy.advise_sell(result, pair=pair)
+    assert type(selldf) is DataFrame
+    assert 'sell' in selldf

user_data/strategies/test_strategy.py

@@ -44,7 +44,7 @@ class TestStrategy(IStrategy):
     # Optimal ticker interval for the strategy
     ticker_interval = '5m'
 
-    def advise_indicators(self, dataframe: DataFrame, pair: str) -> DataFrame:
+    def populate_indicators(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Adds several different TA indicators to the given DataFrame
 
@@ -211,7 +211,7 @@ class TestStrategy(IStrategy):
 
         return dataframe
 
-    def advise_buy(self, dataframe: DataFrame, pair: str) -> DataFrame:
+    def populate_buy_trend(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Based on TA indicators, populates the buy signal for the given dataframe
         :param dataframe: DataFrame populated with indicators
@@ -228,7 +228,7 @@ class TestStrategy(IStrategy):
 
         return dataframe
 
-    def advise_sell(self, dataframe: DataFrame, pair: str) -> DataFrame:
+    def populate_sell_trend(self, dataframe: DataFrame, pair: str) -> DataFrame:
         """
         Based on TA indicators, populates the sell signal for the given dataframe
         :param dataframe: DataFrame populated with indicators