Build out user dir by inverse · Pull Request #31 · ZENALC/algobot

inverse · 2021-07-06T19:57:40Z

Prepare for packaging up as an application by moving user data out of the project repository to the user home directories.

This is a significant change.

Partial work for #29

inverse · 2021-07-08T21:05:07Z

 LONG_INTERVAL_MAP = {v: k for k, v in SHORT_INTERVAL_MAP.items()}


+class Paths:


All path based information is encapsulated in this class

We should probably store this class in another file. With that being said, we should really organize the layout of the project.. Everything is kinda all scattered all across right now

I would advise perhaps doing that in another PR. Doing things incrementally and in small commits is easier. Especially to revert if things go south.

ZENALC

Nice! Thank you for your contribution my friend!

I will have a more thorough review in the next few days and merge

ZENALC · 2021-07-09T01:02:12Z

 from binance.helpers import interval_to_milliseconds

-from algobot.helpers import (ROOT_DIR, get_logger, get_normalized_data,
+from algobot import helpers


Here also, why not just have from algobot.helpers import ...?

Updated all

ZENALC · 2021-07-09T01:02:38Z

            raise ValueError("Invalid interval.", 4)

-    def create_folders_and_change_path(self, folderName: str):
+    def create_folders_and_change_path(self, folder_name: str):


Nice change! We should really convert camel case to snake case over time

snake case is the python way. If you're happy with something we could use black?

https://black.readthedocs.io/en/stable/

We can configure it as a pre-commit hook even :)

Although not sure how it handles variable naming come to think about it 🤔

pylint will help enforce it ;)

Yeap. We should really start leveraging pylint and mypy

ZENALC · 2021-07-09T01:06:39Z

-        folder = os.path.basename(targetPath)
-        os.mkdir(os.path.join(basePath, folder))
+    if not os.path.exists(target_path):
+        os.makedirs(target_path)


We can also use the exist_ok kwarg and set that to True so we don't need to check the conditional

Won't that change the semantics of the function?

As in we return true/false if the directory exists?

ZENALC · 2021-07-09T01:06:52Z



-def open_file_or_folder(targetPath: str):
+def open_file_or_folder(target_path: str):


koutsie

LGTM so far.

inverse · 2021-07-10T20:47:29Z

+
+
+def pytest_unconfigure(config):
+    from algobot.helpers import PATHS, AppDirTemp


Super important this is done here otherwise we init PATHS before setting the env var to control which one gets init

ZENALC · 2021-07-11T03:00:16Z

-        fileName = fileName if fileName != '' else None
+        backtest_folder_path = helpers.PATHS.get_backtest_results_dir()
+        create_folder_if_needed(backtest_folder_path)
+        inner_path = os.path.join(backtest_folder_path, self.backtester.symbol)


We should probably refactor this code to use makedirs()

create_folder_if_needed calls that under the hood.

ZENALC · 2021-07-11T03:07:20Z

 pytest-socket==0.4.0
-flake8==3.9.0
-pre-commit==2.11.1
+flake8==3.9.2


We should migrate to pipenv, so much easier with [dev-packages[ and piplock...

We could do - but lets leave that to another PR ;)

There is also https://python-poetry.org/

I've not used but looks quite nice.

inverse · 2021-07-11T08:23:01Z

    assert parse_strategy_name(name) == expected, f"Expected parsed strategy to be: {expected}."
+
+
+def test_create_folder_if_needed():


@ZENALC Started adding some tests too but didn't wanna go too overkill yet.

Pycharms is reporting this metric

inverse · 2021-07-11T08:35:34Z

            self.infoLabel.setText(f'{directory.capitalize()} files have been successfully deleted.')

-            if directory == 'Logs':  # Reinitialize log folder if old logs were purged.
+            if directory.endswith('Logs'):  # Reinitialize log folder if old logs were purged.


Could prob have this defined as a callback and passed into the caller.

But that's for another PR

inverse added 10 commits July 6, 2021 21:56

Build out user dir

e0a6be0

Rework

1d1d20f

Fix

a0f146c

Wrap

acb1bd9

Lint

eeba548

Refactor

f115f41

Rework more

b8027a1

fix up

ef4a981

Aditional tidy of purge

bca643d

Remove more ROOT calls

71fb806

inverse marked this pull request as ready for review July 8, 2021 21:04

inverse commented Jul 8, 2021

View reviewed changes

Comment thread algobot/helpers.py Outdated

ZENALC reviewed Jul 9, 2021

View reviewed changes

inverse added 4 commits July 9, 2021 21:41

Refactor

a5a6542

Avoid errors

e7af0ac

Tidy

5393bca

Add mocks

ab8ae45

koutsie approved these changes Jul 10, 2021

View reviewed changes

inverse added 2 commits July 10, 2021 22:25

Test case

2c7edb2

Fix conftest + more tests

e3710fb

inverse commented Jul 10, 2021

View reviewed changes

Extend

73b605e

ZENALC reviewed Jul 11, 2021

View reviewed changes

Comment thread algobot/helpers.py Outdated

ZENALC reviewed Jul 11, 2021

View reviewed changes

Comment thread algobot/helpers.py Outdated

ZENALC reviewed Jul 11, 2021

View reviewed changes

PR feedback

b69a08e

inverse commented Jul 11, 2021

View reviewed changes

Fix purge

f2094be

inverse commented Jul 11, 2021

View reviewed changes

inverse added 2 commits July 11, 2021 21:19

Add

9c6737f

Remove helpers

463e9bf

ZENALC self-assigned this Jul 16, 2021

ZENALC added the enhancement New feature or request label Jul 16, 2021

		LONG_INTERVAL_MAP = {v: k for k, v in SHORT_INTERVAL_MAP.items()}


		class Paths:



		def open_file_or_folder(targetPath: str):
		def open_file_or_folder(target_path: str):



		def pytest_unconfigure(config):
		from algobot.helpers import PATHS, AppDirTemp

		assert parse_strategy_name(name) == expected, f"Expected parsed strategy to be: {expected}."


		def test_create_folder_if_needed():

Conversation

inverse commented Jul 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ZENALC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inverse Jul 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koutsie left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

inverse commented Jul 6, 2021 •

edited

Loading

inverse Jul 9, 2021 •

edited

Loading

koutsie left a comment •

edited

Loading