Include Commissions in _trades DataFrame. #1276
Conversation
| 'SL': [t.sl for t in trades], | ||
| 'TP': [t.tp for t in trades], | ||
| 'PnL': [t.pl for t in trades], | ||
| 'Commissions': [t._commissions for t in trades], |
There was a problem hiding this comment.
Reasonable! Can we put this in conditionally, only if commission= was passed non-0 which is the default?
There was a problem hiding this comment.
I realize this would make sense for 'SL' and 'TP' too ...
There was a problem hiding this comment.
Also, PnL doesn't account for Commissions. Rationale behind this decision?
There was a problem hiding this comment.
If you're implying PnL should be reduced for the respective value of Commissions, I guess it might be a bug!
There was a problem hiding this comment.
Yes, PnL should ideally account for Commissions and Spread. I can make the fix if you're aligned.
There was a problem hiding this comment.
Absolutely! Ideally, also document it if you can, maybe here. Particularly if there was to become a discrepancy between what trade.pl and this data frame reported.
Also think there might be a few open bugs related to commissions computation. Maybe see if you can get some of those by the way? 😅
There was a problem hiding this comment.
Will push these fixes :)
|
Incrementally taking up the changes. Please review this CR which includes commissions as the first step. |
2 participants
Include
Commissionsin_tradesDataFrame.