Merge pull request #3619 from hroff-1902/cleanup_agefilter

Cleanup AgeFilter, PriceFilter

docs/configuration.md

@@ -663,24 +663,28 @@ Filters low-value coins which would not allow setting stoplosses.
 #### PriceFilter
 
 The `PriceFilter` allows filtering of pairs by price. Currently the following price filters are supported:
+
 * `min_price`
 * `max_price`
 * `low_price_ratio`
 
 The `min_price` setting removes pairs where the price is below the specified price. This is useful if you wish to avoid trading very low-priced pairs.
-This option is disabled by default, and will only apply if set to <> 0.
+This option is disabled by default, and will only apply if set to > 0.
 
 The `max_price` setting removes pairs where the price is above the specified price. This is useful if you wish to trade only low-priced pairs.
-This option is disabled by default, and will only apply if set to <> 0.
+This option is disabled by default, and will only apply if set to > 0.
 
 The `low_price_ratio` setting removes pairs where a raise of 1 price unit (pip) is above the `low_price_ratio` ratio.
-This option is disabled by default, and will only apply if set to <> 0.
+This option is disabled by default, and will only apply if set to > 0.
+
+For `PriceFiler` at least one of its `min_price`, `max_price` or `low_price_ratio` settings must be applied.
 
 Calculation example:
 
-Min price precision is 8 decimals. If price is 0.00000011 - one step would be 0.00000012 - which is almost 10% higher than the previous value.
+Min price precision for SHITCOIN/BTC is 8 decimals. If its price is 0.00000011 - one price step above would be 0.00000012, which is ~9% higher than the previous price value. You may filter out this pair by using PriceFilter with `low_price_ratio` set to 0.09 (9%) or with `min_price` set to 0.00000011, correspondingly.
 
-These pairs are dangerous since it may be impossible to place the desired stoploss - and often result in high losses.
+!!! Warning "Low priced pairs"
+    Low priced pairs with high "1 pip movements" are dangerous since they are often illiquid and it may also be impossible to place the desired stoploss, which can often result in high losses since price needs to be rounded to the next tradable price - so instead of having a stoploss of -5%, you could end up with a stoploss of -9% simply due to price rounding.
 
 #### ShuffleFilter
 

freqtrade/pairlist/AgeFilter.py

@@ -26,12 +26,11 @@ class AgeFilter(IPairList):
         self._min_days_listed = pairlistconfig.get('min_days_listed', 10)
 
         if self._min_days_listed < 1:
-            raise OperationalException("AgeFilter requires min_days_listed must be >= 1")
+            raise OperationalException("AgeFilter requires min_days_listed to be >= 1")
         if self._min_days_listed > exchange.ohlcv_candle_limit:
-            raise OperationalException("AgeFilter requires min_days_listed must not exceed "
+            raise OperationalException("AgeFilter requires min_days_listed to not exceed "
                                        "exchange max request size "
                                        f"({exchange.ohlcv_candle_limit})")
-        self._enabled = self._min_days_listed >= 1
 
     @property
     def needstickers(self) -> bool:

freqtrade/pairlist/PriceFilter.py

@@ -4,6 +4,7 @@ Price pair list filter
 import logging
 from typing import Any, Dict
 
+from freqtrade.exceptions import OperationalException
 from freqtrade.pairlist.IPairList import IPairList
 
 
@@ -18,11 +19,17 @@ class PriceFilter(IPairList):
         super().__init__(exchange, pairlistmanager, config, pairlistconfig, pairlist_pos)
 
         self._low_price_ratio = pairlistconfig.get('low_price_ratio', 0)
+        if self._low_price_ratio < 0:
+            raise OperationalException("PriceFilter requires low_price_ratio to be >= 0")
         self._min_price = pairlistconfig.get('min_price', 0)
+        if self._min_price < 0:
+            raise OperationalException("PriceFilter requires min_price to be >= 0")
         self._max_price = pairlistconfig.get('max_price', 0)
-        self._enabled = ((self._low_price_ratio != 0) or
+        if self._max_price < 0:
-                         (self._min_price != 0) or
+            raise OperationalException("PriceFilter requires max_price to be >= 0")
-                         (self._max_price != 0))
+        self._enabled = ((self._low_price_ratio > 0) or
+                         (self._min_price > 0) or
+                         (self._max_price > 0))
 
     @property
     def needstickers(self) -> bool:

tests/pairlist/test_pairlist.py

@@ -549,7 +549,7 @@ def test_agefilter_min_days_listed_too_small(mocker, default_conf, markets, tick
                           )
 
     with pytest.raises(OperationalException,
-                       match=r'AgeFilter requires min_days_listed must be >= 1'):
+                       match=r'AgeFilter requires min_days_listed to be >= 1'):
         get_patched_freqtradebot(mocker, default_conf)
 
 
@@ -564,7 +564,7 @@ def test_agefilter_min_days_listed_too_large(mocker, default_conf, markets, tick
                           )
 
     with pytest.raises(OperationalException,
-                       match=r'AgeFilter requires min_days_listed must not exceed '
+                       match=r'AgeFilter requires min_days_listed to not exceed '
                              r'exchange max request size \([0-9]+\)'):
         get_patched_freqtradebot(mocker, default_conf)
 
@@ -592,34 +592,58 @@ def test_agefilter_caching(mocker, markets, whitelist_conf_3, tickers, ohlcv_his
     assert freqtrade.exchange.get_historic_ohlcv.call_count == previous_call_count
 
 
-@pytest.mark.parametrize("pairlistconfig,expected", [
+@pytest.mark.parametrize("pairlistconfig,desc_expected,exception_expected", [
     ({"method": "PriceFilter", "low_price_ratio": 0.001, "min_price": 0.00000010,
-      "max_price": 1.0}, "[{'PriceFilter': 'PriceFilter - Filtering pairs priced below "
+      "max_price": 1.0},
-                         "0.1% or below 0.00000010 or above 1.00000000.'}]"
+     "[{'PriceFilter': 'PriceFilter - Filtering pairs priced below "
+     "0.1% or below 0.00000010 or above 1.00000000.'}]",
+     None
      ),
     ({"method": "PriceFilter", "low_price_ratio": 0.001, "min_price": 0.00000010},
-     "[{'PriceFilter': 'PriceFilter - Filtering pairs priced below 0.1% or below 0.00000010.'}]"
+     "[{'PriceFilter': 'PriceFilter - Filtering pairs priced below 0.1% or below 0.00000010.'}]",
+     None
      ),
     ({"method": "PriceFilter", "low_price_ratio": 0.001, "max_price": 1.00010000},
-     "[{'PriceFilter': 'PriceFilter - Filtering pairs priced below 0.1% or above 1.00010000.'}]"
+     "[{'PriceFilter': 'PriceFilter - Filtering pairs priced below 0.1% or above 1.00010000.'}]",
+     None
      ),
     ({"method": "PriceFilter", "min_price": 0.00002000},
-     "[{'PriceFilter': 'PriceFilter - Filtering pairs priced below 0.00002000.'}]"
+     "[{'PriceFilter': 'PriceFilter - Filtering pairs priced below 0.00002000.'}]",
+     None
      ),
     ({"method": "PriceFilter"},
-     "[{'PriceFilter': 'PriceFilter - No price filters configured.'}]"
+     "[{'PriceFilter': 'PriceFilter - No price filters configured.'}]",
+     None
      ),
+    ({"method": "PriceFilter", "low_price_ratio": -0.001},
+     None,
+     "PriceFilter requires low_price_ratio to be >= 0"
+     ),  # OperationalException expected
+    ({"method": "PriceFilter", "min_price": -0.00000010},
+     None,
+     "PriceFilter requires min_price to be >= 0"
+     ),  # OperationalException expected
+    ({"method": "PriceFilter", "max_price": -1.00010000},
+     None,
+     "PriceFilter requires max_price to be >= 0"
+     ),  # OperationalException expected
 ])
-def test_pricefilter_desc(mocker, whitelist_conf, markets, pairlistconfig, expected):
+def test_pricefilter_desc(mocker, whitelist_conf, markets, pairlistconfig,
+                          desc_expected, exception_expected):
     mocker.patch.multiple('freqtrade.exchange.Exchange',
                           markets=PropertyMock(return_value=markets),
                           exchange_has=MagicMock(return_value=True)
                           )
     whitelist_conf['pairlists'] = [pairlistconfig]
 
+    if desc_expected is not None:
         freqtrade = get_patched_freqtradebot(mocker, whitelist_conf)
         short_desc = str(freqtrade.pairlists.short_desc())
-    assert short_desc == expected
+        assert short_desc == desc_expected
+    else:  # OperationalException expected
+        with pytest.raises(OperationalException,
+                           match=exception_expected):
+            freqtrade = get_patched_freqtradebot(mocker, whitelist_conf)
 
 
 def test_pairlistmanager_no_pairlist(mocker, markets, whitelist_conf, caplog):