don't use DEFAULT_CANDLES if Hexital.timeframe is given #49
Conversation
powellnorma
force-pushed
the
opt_default
branch
from
7a8462f to
076dad0
Compare
| @@ -32,6 +32,8 @@ class Hexital: | |||
| candle_life: Optional[timedelta] = None | |||
| candlestick: Optional[CandlestickType] = None | |||
|
|
|||
| default_tf: str = DEFAULT_CANDLES | |||
There was a problem hiding this comment.
Let's make this a private attribute and rename it -> _timeframe_name
This is because it's just _timeframe but the str version. Having it duplicated just means we dont need to call timedelta_to_str or convert_timeframe_to_timedelta regularly.
We make it private because there is a timeframe property to get this information out, we should update this too.
| @@ -53,16 +55,19 @@ def __init__( | |||
| if candlestick: | |||
| self.candlestick = validate_candlesticktype(candlestick) | |||
|
|
|||
| if self._timeframe is not None: | |||
| self.default_tf = self.timeframe | |||
There was a problem hiding this comment.
I dont think this is correct, self.timeframe does not exist. We parse timeframe into self._timeframe. As this is just the name version of timeframe we should have:
self._timeframe_name = timedelta_to_str(self._timeframe)| candles if isinstance(candles, list) else [], | ||
| candle_life=self.candle_life, | ||
| timeframe=self._timeframe, | ||
| timeframe_fill=self.timeframe_fill, | ||
| candlestick=self.candlestick, | ||
| ) | ||
| } | ||
| self._candles[DEFAULT_CANDLES].name = DEFAULT_CANDLES | ||
| self._candles[self.default_tf].name = self.default_tf |
There was a problem hiding this comment.
I dont think we need this line at all, not your fault of course.
There was a problem hiding this comment.
The property
def timeframe(self) -> str | None:
return timedelta_to_str(self._timeframe) if self._timeframe else NoneCan just be updated now:
def timeframe(self) -> str | None:
return self._timeframe_name if self._timeframe else None
|
I like this fix, simple and affective, we are effectivly saving the
class TestMultiTimeframes:
def test_timeframe_default(self, candles):
strat = Hexital("Test Strategy", candles)
assert list(strat._candles.keys()) == ["default"]
def test_timeframe_multi(self, candles):
strat = Hexital("Test Strategy", candles, [EMA(timeframe="T1")])
assert list(strat._candles.keys()) == ["default", "T1"]
def test_duplicate_indicators(self, candles):
strat = Hexital("Test Strategy", candles, [EMA(timeframe="T1"), SMA(timeframe="T1")])
assert list(strat._candles.keys()) == ["default", "T1"]
def test_clash_hexital(self, candles):
strat = Hexital("Test Strategy", candles, [EMA(timeframe="T1")], timeframe="T1")
assert list(strat._candles.keys()) == ["T1"]
def test_multi_hexital(self, candles):
strat = Hexital("Test Strategy", candles, [EMA(timeframe="T5")], timeframe="T1")
assert list(strat._candles.keys()) == ["T1", "T5"] |
|
Thanks for the review! I’ve enabled 'Allow edits by maintainers,' so feel free to commit the changes directly to the branch. Let me know if you need anything else. |
|
Pr been abandoned |
No description provided.