Encapsulate Trailing and Stop #43
Conversation
| TRAILING = 2 | ||
| STOP = 1 | ||
|
|
||
| class OrderType: |
There was a problem hiding this comment.
Still could be improved but it's a step in the right direction. by improvements we could use real enums and get read of the whole from_str but that would require a lot more mapping ;)
Like mapping the UI element to string -> enum value :)
algobot/interface/configuration.py
Outdated
| @@ -73,7 +73,7 @@ def __init__(self, parent: QMainWindow, logger: Logger = None): | |||
| } | |||
|
|
|||
| self.lossTypes = ("Trailing", "Stop") | |||
There was a problem hiding this comment.
we should probably leverage the actual enum values? let's not use integers but actual strings? what do you think? :)
There was a problem hiding this comment.
Could work - trying to think of how would be best to handle semantics vs presentation
There was a problem hiding this comment.
Well, as long as the enums stay the same, we can just replace the value.
So TRAILING can change from 1 to "Trailing", then we don't even need a mapper ;p
algobot/interface/configuration.py
Outdated
| @@ -343,16 +343,16 @@ def get_take_profit_settings(self, caller) -> dict: | |||
| tab = self.get_category_tab(caller) | |||
| dictionary = self.takeProfitDict | |||
| if dictionary[tab, 'groupBox'].isChecked(): | |||
| if dictionary[tab, 'takeProfitType'].currentText() == "Trailing": | |||
| takeProfitType = TRAILING | |||
| if dictionary[tab, 'takeOrderType'].currentText() == "Trailing": | |||
There was a problem hiding this comment.
we should probably use the to_str method()
| @@ -246,9 +246,9 @@ def get_stop_loss_strategy_string(self) -> str: | |||
| Returns stop loss strategy in string format, instead of integer enum. | |||
| :return: Stop loss strategy in string format. | |||
| """ | |||
| if self.lossStrategy == STOP: | |||
| if self.lossStrategy == OrderType.STOP: | |||
There was a problem hiding this comment.
We should probably deprecate this function and add a kwarg for suffix in the to_str method
tests/test_backtester.py
Outdated
| self.assertEqual(backtester.get_stop_loss(), 3 * (1 + backtester.lossPercentageDecimal)) | ||
|
|
||
| def test_stop_take_profit(self): | ||
| """ | ||
| Test backtester take profit logic. | ||
| """ | ||
| backtester = self.backtester | ||
| backtester.takeProfitType = STOP | ||
| backtester.takeOrderType = OrderType.STOP |
| from algobot.enums import OrderType | ||
|
|
||
|
|
||
| class OrderTypeTest(unittest.TestCase): |
There was a problem hiding this comment.
let's use pytest? We should soon start rewriting all unit test tests to leverage pytest :p
Encapsulate
TRAILINGandSTOPinto distinct objects.Not 100% sure of this one so please correct me where I mis-interpreted things.