Refactor of _optimize_assets_for_exchange #269
Conversation
|
|
||
| # Copy over assets from previous timestamps so there are no holes in the graph | ||
| composite_optimizations: Dict[datetime, Dict[str, Dict[str, float]]] = {} | ||
| previous_timestamp: Optional[datetime] = None | ||
|
|
||
| for timestamp, optimization_dict in sorted_optimizations.items(): | ||
| if previous_timestamp is None: | ||
| composite_optimizations[timestamp] = optimization_dict | ||
| else: | ||
| composite_optimizations[timestamp] = sorted_optimizations[previous_timestamp].copy() | ||
| composite_optimizations[timestamp].update(sorted_optimizations[timestamp]) | ||
|
|
There was a problem hiding this comment.
This code was cut out since the layering process was causing errors and unexpected behavior. It's actually not really necessary since we delete duplicates later.
| timestamp_diff: float = (child_bars[child_name][neighbor.name][0].timestamp - start_date).total_seconds() | ||
|
|
||
| # Find the start of the market if it is after the first transaction | ||
| if timestamp_diff > _TIME_GRANULARITY_STRING_TO_SECONDS[_ONE_WEEK]: | ||
| market_starts[child_name][neighbor.name] = child_bars[child_name][neighbor.name][0].timestamp |
There was a problem hiding this comment.
This was causing the no_market_padding bar to never be used, so I removed it.
|
@eprbell Most of the code was simply moved into individual functions except the two pieces I commented on above. |
Sounds good: I'm planning to review this weekend. |
Yeah, I'd like to do this for the core functions. It is a complicated beast and I found two bugs just in this one function. |
What?
This is essentially a refactoring of
_optimize_assets_for_exchangeI broke it into 6 stages and created individual functions for most of them to test them individually. This resulted in finding two bugs.
Questions
I'm not sure if I should make these individual stage functions private or extract this logic into a separate module of some kind.