From 89db5c6bab15352c334e79bf0cba2ab720cd9687 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 21 Jul 2019 14:52:59 +0200 Subject: [PATCH 1/8] Extract strategy-specific stuff from search logic will allow extracting all to IResolver --- freqtrade/resolvers/strategy_resolver.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/freqtrade/resolvers/strategy_resolver.py b/freqtrade/resolvers/strategy_resolver.py index 4a5604db8..114115d8a 100644 --- a/freqtrade/resolvers/strategy_resolver.py +++ b/freqtrade/resolvers/strategy_resolver.py @@ -155,19 +155,23 @@ class StrategyResolver(IResolver): kwargs={'config': config}) if strategy: logger.info(f"Using resolved strategy {strategy_name} from '{module_path}'...") - strategy._populate_fun_len = len( - getfullargspec(strategy.populate_indicators).args) - strategy._buy_fun_len = len(getfullargspec(strategy.populate_buy_trend).args) - strategy._sell_fun_len = len(getfullargspec(strategy.populate_sell_trend).args) - try: - return import_strategy(strategy, config=config) - except TypeError as e: - logger.warning( - f"Impossible to load strategy '{strategy_name}' from {module_path}. " - f"Error: {e}") + break + except FileNotFoundError: logger.warning('Path "%s" does not exist.', _path.relative_to(Path.cwd())) + if strategy: + strategy._populate_fun_len = len(getfullargspec(strategy.populate_indicators).args) + strategy._buy_fun_len = len(getfullargspec(strategy.populate_buy_trend).args) + strategy._sell_fun_len = len(getfullargspec(strategy.populate_sell_trend).args) + + try: + return import_strategy(strategy, config=config) + except TypeError as e: + logger.warning( + f"Impossible to load strategy '{strategy_name}'. " + f"Error: {e}") + raise OperationalException( f"Impossible to load Strategy '{strategy_name}'. This class does not exist " "or contains Python code errors." From b35efd96dc01eb0e1b436577c778b59688ed4d04 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 21 Jul 2019 15:03:12 +0200 Subject: [PATCH 2/8] Extract load_object from multiple paths to iResolver --- freqtrade/resolvers/hyperopt_resolver.py | 34 +++++++++--------------- freqtrade/resolvers/iresolver.py | 25 ++++++++++++++++- freqtrade/resolvers/pairlist_resolver.py | 18 +++++-------- freqtrade/resolvers/strategy_resolver.py | 17 +++--------- 4 files changed, 46 insertions(+), 48 deletions(-) diff --git a/freqtrade/resolvers/hyperopt_resolver.py b/freqtrade/resolvers/hyperopt_resolver.py index 42e5ff31c..3f8d03fd2 100644 --- a/freqtrade/resolvers/hyperopt_resolver.py +++ b/freqtrade/resolvers/hyperopt_resolver.py @@ -63,17 +63,12 @@ class HyperOptResolver(IResolver): # Add extra hyperopt directory on top of search paths abs_paths.insert(0, Path(extra_dir)) - for _path in abs_paths: - try: - (hyperopt, module_path) = self._search_object(directory=_path, - object_type=IHyperOpt, - object_name=hyperopt_name) - if hyperopt: - logger.info(f"Using resolved hyperopt {hyperopt_name} from '{module_path}'...") - return hyperopt - except FileNotFoundError: - logger.warning('Path "%s" does not exist.', _path.relative_to(Path.cwd())) - + (hyperopt, module_path) = self._load_object(paths=abs_paths, + object_type=IHyperOpt, + object_name=hyperopt_name, + kwargs={}) + if hyperopt: + return hyperopt raise OperationalException( f"Impossible to load Hyperopt '{hyperopt_name}'. This class does not exist " "or contains Python code errors." @@ -125,17 +120,12 @@ class HyperOptLossResolver(IResolver): # Add extra hyperopt directory on top of search paths abs_paths.insert(0, Path(extra_dir)) - for _path in abs_paths: - try: - (hyperoptloss, module_path) = self._search_object(directory=_path, - object_type=IHyperOptLoss, - object_name=hyper_loss_name) - if hyperoptloss: - logger.info( - f"Using resolved hyperopt {hyper_loss_name} from '{module_path}'...") - return hyperoptloss - except FileNotFoundError: - logger.warning('Path "%s" does not exist.', _path.relative_to(Path.cwd())) + (hyperoptloss, module_path) = self._load_object(paths=abs_paths, + object_type=IHyperOptLoss, + object_name=hyper_loss_name, + kwargs={}) + if hyperoptloss: + return hyperoptloss raise OperationalException( f"Impossible to load HyperoptLoss '{hyper_loss_name}'. This class does not exist " diff --git a/freqtrade/resolvers/iresolver.py b/freqtrade/resolvers/iresolver.py index 1065abba7..aafc4b0dd 100644 --- a/freqtrade/resolvers/iresolver.py +++ b/freqtrade/resolvers/iresolver.py @@ -7,7 +7,7 @@ import importlib.util import inspect import logging from pathlib import Path -from typing import Any, Optional, Tuple, Type, Union +from typing import Any, List, Optional, Tuple, Type, Union logger = logging.getLogger(__name__) @@ -64,3 +64,26 @@ class IResolver(object): if obj: return (obj(**kwargs), module_path) return (None, None) + + @staticmethod + def _load_object(paths: List[Path], object_type, object_name: str, + kwargs: dict = {}) -> Union[Tuple[Any, Path], Tuple[None, None]]: + """ + Try to load object from path list. + """ + + for _path in paths: + try: + (module, module_path) = IResolver._search_object(directory=_path, + object_type=object_type, + object_name=object_name, + kwargs=kwargs) + if module: + logger.info( + f"Using resolved {object_type.__name__.lower()[1:]} {object_name} " + f"from '{module_path}'...") + return (module, module_path) + except FileNotFoundError: + logger.warning('Path "%s" does not exist.', _path.relative_to(Path.cwd())) + + return (None, None) diff --git a/freqtrade/resolvers/pairlist_resolver.py b/freqtrade/resolvers/pairlist_resolver.py index 651a7d33f..a74ce1cf7 100644 --- a/freqtrade/resolvers/pairlist_resolver.py +++ b/freqtrade/resolvers/pairlist_resolver.py @@ -43,18 +43,12 @@ class PairListResolver(IResolver): current_path, ] - for _path in abs_paths: - try: - (pairlist, module_path) = self._search_object(directory=_path, - object_type=IPairList, - object_name=pairlist_name, - kwargs=kwargs) - if pairlist: - logger.info(f"Using resolved pairlist {pairlist_name} from '{module_path}'...") - return pairlist - except FileNotFoundError: - logger.warning('Path "%s" does not exist.', _path.relative_to(Path.cwd())) - + (pairlist, module_path) = self._load_object(paths=abs_paths, + object_type=IPairList, + object_name=pairlist_name, + kwargs=kwargs) + if pairlist: + return pairlist raise OperationalException( f"Impossible to load Pairlist '{pairlist_name}'. This class does not exist " "or contains Python code errors." diff --git a/freqtrade/resolvers/strategy_resolver.py b/freqtrade/resolvers/strategy_resolver.py index 114115d8a..ac053399e 100644 --- a/freqtrade/resolvers/strategy_resolver.py +++ b/freqtrade/resolvers/strategy_resolver.py @@ -147,19 +147,10 @@ class StrategyResolver(IResolver): # register temp path with the bot abs_paths.insert(0, temp.resolve()) - for _path in abs_paths: - try: - (strategy, module_path) = self._search_object(directory=_path, - object_type=IStrategy, - object_name=strategy_name, - kwargs={'config': config}) - if strategy: - logger.info(f"Using resolved strategy {strategy_name} from '{module_path}'...") - break - - except FileNotFoundError: - logger.warning('Path "%s" does not exist.', _path.relative_to(Path.cwd())) - + (strategy, module_path) = self._load_object(paths=abs_paths, + object_type=IStrategy, + object_name=strategy_name, + kwargs={'config': config}) if strategy: strategy._populate_fun_len = len(getfullargspec(strategy.populate_indicators).args) strategy._buy_fun_len = len(getfullargspec(strategy.populate_buy_trend).args) From 88eb93da52421ffa74ab5c5a6ad16aceb90b72cc Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 21 Jul 2019 15:16:19 +0200 Subject: [PATCH 3/8] Fix base64 strategy test to make sure strategy was loaded via base64 --- freqtrade/tests/strategy/test_strategy.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/freqtrade/tests/strategy/test_strategy.py b/freqtrade/tests/strategy/test_strategy.py index 9a2c950e5..02a3769fc 100644 --- a/freqtrade/tests/strategy/test_strategy.py +++ b/freqtrade/tests/strategy/test_strategy.py @@ -1,5 +1,6 @@ # pragma pylint: disable=missing-docstring, protected-access, C0103 import logging +import tempfile import warnings from base64 import urlsafe_b64encode from os import path @@ -68,11 +69,15 @@ def test_load_strategy(result): assert 'adx' in resolver.strategy.advise_indicators(result, {'pair': 'ETH/BTC'}) -def test_load_strategy_base64(result): - with open("freqtrade/tests/strategy/test_strategy.py", "rb") as file: +def test_load_strategy_base64(result, caplog): + with open("user_data/strategies/test_strategy.py", "rb") as file: encoded_string = urlsafe_b64encode(file.read()).decode("utf-8") resolver = StrategyResolver({'strategy': 'TestStrategy:{}'.format(encoded_string)}) assert 'adx' in resolver.strategy.advise_indicators(result, {'pair': 'ETH/BTC'}) + # Make sure strategy was loaded from base64 (using temp directory)!! + assert log_has_re(r"Using resolved strategy TestStrategy from '" + + tempfile.gettempdir() + r"/.*/TestStrategy\.py'\.\.\.", + caplog.record_tuples) def test_load_strategy_invalid_directory(result, caplog): From 08ca260e82473f3630ce4dacb8f49fbef839b275 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 21 Jul 2019 15:25:48 +0200 Subject: [PATCH 4/8] Simplify return valuef rom _load_object --- freqtrade/resolvers/hyperopt_resolver.py | 12 ++++-------- freqtrade/resolvers/iresolver.py | 6 +++--- freqtrade/resolvers/pairlist_resolver.py | 6 ++---- freqtrade/resolvers/strategy_resolver.py | 6 ++---- 4 files changed, 11 insertions(+), 19 deletions(-) diff --git a/freqtrade/resolvers/hyperopt_resolver.py b/freqtrade/resolvers/hyperopt_resolver.py index 3f8d03fd2..3f39bd41d 100644 --- a/freqtrade/resolvers/hyperopt_resolver.py +++ b/freqtrade/resolvers/hyperopt_resolver.py @@ -63,10 +63,8 @@ class HyperOptResolver(IResolver): # Add extra hyperopt directory on top of search paths abs_paths.insert(0, Path(extra_dir)) - (hyperopt, module_path) = self._load_object(paths=abs_paths, - object_type=IHyperOpt, - object_name=hyperopt_name, - kwargs={}) + hyperopt = self._load_object(paths=abs_paths, object_type=IHyperOpt, + object_name=hyperopt_name) if hyperopt: return hyperopt raise OperationalException( @@ -120,10 +118,8 @@ class HyperOptLossResolver(IResolver): # Add extra hyperopt directory on top of search paths abs_paths.insert(0, Path(extra_dir)) - (hyperoptloss, module_path) = self._load_object(paths=abs_paths, - object_type=IHyperOptLoss, - object_name=hyper_loss_name, - kwargs={}) + hyperoptloss = self._load_object(paths=abs_paths, object_type=IHyperOptLoss, + object_name=hyper_loss_name) if hyperoptloss: return hyperoptloss diff --git a/freqtrade/resolvers/iresolver.py b/freqtrade/resolvers/iresolver.py index aafc4b0dd..192a38beb 100644 --- a/freqtrade/resolvers/iresolver.py +++ b/freqtrade/resolvers/iresolver.py @@ -67,7 +67,7 @@ class IResolver(object): @staticmethod def _load_object(paths: List[Path], object_type, object_name: str, - kwargs: dict = {}) -> Union[Tuple[Any, Path], Tuple[None, None]]: + kwargs: dict = {}) -> Union[Any, None]: """ Try to load object from path list. """ @@ -82,8 +82,8 @@ class IResolver(object): logger.info( f"Using resolved {object_type.__name__.lower()[1:]} {object_name} " f"from '{module_path}'...") - return (module, module_path) + return module except FileNotFoundError: logger.warning('Path "%s" does not exist.', _path.relative_to(Path.cwd())) - return (None, None) + return None diff --git a/freqtrade/resolvers/pairlist_resolver.py b/freqtrade/resolvers/pairlist_resolver.py index a74ce1cf7..3d95c0295 100644 --- a/freqtrade/resolvers/pairlist_resolver.py +++ b/freqtrade/resolvers/pairlist_resolver.py @@ -43,10 +43,8 @@ class PairListResolver(IResolver): current_path, ] - (pairlist, module_path) = self._load_object(paths=abs_paths, - object_type=IPairList, - object_name=pairlist_name, - kwargs=kwargs) + pairlist = self._load_object(paths=abs_paths, object_type=IPairList, + object_name=pairlist_name, kwargs=kwargs) if pairlist: return pairlist raise OperationalException( diff --git a/freqtrade/resolvers/strategy_resolver.py b/freqtrade/resolvers/strategy_resolver.py index ac053399e..aa73327ff 100644 --- a/freqtrade/resolvers/strategy_resolver.py +++ b/freqtrade/resolvers/strategy_resolver.py @@ -147,10 +147,8 @@ class StrategyResolver(IResolver): # register temp path with the bot abs_paths.insert(0, temp.resolve()) - (strategy, module_path) = self._load_object(paths=abs_paths, - object_type=IStrategy, - object_name=strategy_name, - kwargs={'config': config}) + strategy = self._load_object(paths=abs_paths, object_type=IStrategy, + object_name=strategy_name, kwargs={'config': config}) if strategy: strategy._populate_fun_len = len(getfullargspec(strategy.populate_indicators).args) strategy._buy_fun_len = len(getfullargspec(strategy.populate_buy_trend).args) From e6528be63d3204c1a7c16fe15cff517d820419bf Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 21 Jul 2019 16:20:45 +0200 Subject: [PATCH 5/8] Config is not optional for hyperopt resolver --- freqtrade/resolvers/hyperopt_resolver.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/freqtrade/resolvers/hyperopt_resolver.py b/freqtrade/resolvers/hyperopt_resolver.py index 3f39bd41d..944687ce7 100644 --- a/freqtrade/resolvers/hyperopt_resolver.py +++ b/freqtrade/resolvers/hyperopt_resolver.py @@ -23,12 +23,11 @@ class HyperOptResolver(IResolver): __slots__ = ['hyperopt'] - def __init__(self, config: Optional[Dict] = None) -> None: + def __init__(self, config: Dict) -> None: """ Load the custom class from config parameter :param config: configuration dictionary or None """ - config = config or {} # Verify the hyperopt is in the configuration, otherwise fallback to the default hyperopt hyperopt_name = config.get('hyperopt') or DEFAULT_HYPEROPT From dcddfce5bcb297d197511659249fd8bff61056a1 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 21 Jul 2019 19:21:50 +0200 Subject: [PATCH 6/8] Fix small mistakes --- freqtrade/resolvers/hyperopt_resolver.py | 2 +- freqtrade/resolvers/iresolver.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/freqtrade/resolvers/hyperopt_resolver.py b/freqtrade/resolvers/hyperopt_resolver.py index 944687ce7..74412e738 100644 --- a/freqtrade/resolvers/hyperopt_resolver.py +++ b/freqtrade/resolvers/hyperopt_resolver.py @@ -26,7 +26,7 @@ class HyperOptResolver(IResolver): def __init__(self, config: Dict) -> None: """ Load the custom class from config parameter - :param config: configuration dictionary or None + :param config: configuration dictionary """ # Verify the hyperopt is in the configuration, otherwise fallback to the default hyperopt diff --git a/freqtrade/resolvers/iresolver.py b/freqtrade/resolvers/iresolver.py index 192a38beb..9d0c97917 100644 --- a/freqtrade/resolvers/iresolver.py +++ b/freqtrade/resolvers/iresolver.py @@ -67,7 +67,7 @@ class IResolver(object): @staticmethod def _load_object(paths: List[Path], object_type, object_name: str, - kwargs: dict = {}) -> Union[Any, None]: + kwargs: dict = {}) -> Optional[Any]: """ Try to load object from path list. """ From 1fea6d394a6cee4227c678485da1776ea38858bd Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 21 Jul 2019 19:31:50 +0200 Subject: [PATCH 7/8] Import DefaultStrategy from the correct file --- freqtrade/strategy/__init__.py | 2 -- freqtrade/tests/strategy/test_strategy.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/freqtrade/strategy/__init__.py b/freqtrade/strategy/__init__.py index c62bfe5dc..19eacda42 100644 --- a/freqtrade/strategy/__init__.py +++ b/freqtrade/strategy/__init__.py @@ -3,8 +3,6 @@ import sys from copy import deepcopy from freqtrade.strategy.interface import IStrategy -# Import Default-Strategy to have hyperopt correctly resolve -from freqtrade.strategy.default_strategy import DefaultStrategy # noqa: F401 logger = logging.getLogger(__name__) diff --git a/freqtrade/tests/strategy/test_strategy.py b/freqtrade/tests/strategy/test_strategy.py index 02a3769fc..609cc58ff 100644 --- a/freqtrade/tests/strategy/test_strategy.py +++ b/freqtrade/tests/strategy/test_strategy.py @@ -45,7 +45,7 @@ def test_import_strategy(caplog): def test_search_strategy(): default_config = {} - default_location = Path(__file__).parent.parent.joinpath('strategy').resolve() + default_location = Path(__file__).parent.parent.parent.joinpath('strategy').resolve() s, _ = StrategyResolver._search_object( directory=default_location, From d2ad32eef8e450222d91ad680b5943c45002f314 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 21 Jul 2019 19:56:43 +0200 Subject: [PATCH 8/8] partially revert last commit(DefaultStrategy import IS needed). * don't run functions in travis in a way we don't support --- .travis.yml | 4 ++-- freqtrade/strategy/__init__.py | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 33a45b280..b44fef7a1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,11 +32,11 @@ jobs: name: pytest - script: - cp config.json.example config.json - - python freqtrade --datadir freqtrade/tests/testdata backtesting + - freqtrade --datadir freqtrade/tests/testdata backtesting name: backtest - script: - cp config.json.example config.json - - python freqtrade --datadir freqtrade/tests/testdata hyperopt -e 5 + - freqtrade --datadir freqtrade/tests/testdata hyperopt -e 5 name: hyperopt - script: flake8 freqtrade scripts name: flake8 diff --git a/freqtrade/strategy/__init__.py b/freqtrade/strategy/__init__.py index 19eacda42..c62bfe5dc 100644 --- a/freqtrade/strategy/__init__.py +++ b/freqtrade/strategy/__init__.py @@ -3,6 +3,8 @@ import sys from copy import deepcopy from freqtrade.strategy.interface import IStrategy +# Import Default-Strategy to have hyperopt correctly resolve +from freqtrade.strategy.default_strategy import DefaultStrategy # noqa: F401 logger = logging.getLogger(__name__)