Add tagging, object typing, fix pep8
Hello again. Many apologies for my butchering of the PR process. Git is certainly something I am no expert at. :cry:
This is another PR which implements the tagging feature, makes tags an object type and fixes previous pep8 issues.
I am currently testing this for the first time on my own trading platform. The code seems to work fine to persist tags at the moment, but there are far too many trades being taken out, so unless that's a likely outcome of these changes, that problem is likely with my code outside of backtesting.py.
@kernc this PR is finally passing all checks now that I configured my PEP8 linter as you have it.
I have made all changes that you referenced in #199 except the sample usage and unit tests. Sorry, I've not had time to do this yet.
The question of what to do with orders that close trades - this refers to the FIFO rule in non-hedging mode, yes?
The changes I've made do seem to work with my own trade simulation code, but I'm still working out a few bugs and would want to work those out before recommending any merge of this to master.
Looks good to me! The tag can function like an id to identify specific trades correct? And would unit testing be helpful here or would it be redundant?
Indeed, it's a way to carry over arbitrary info from orders to trades.
Thanks for the reminder about tests. Testing at least that tag= indeed carries over to the trade would be appropriate. That's a simple, fast, couple-line coroutine-based test:
https://github.com/kernc/backtesting.py/blob/73e1534428fc2db9e681030862133b5067508520/backtesting/test/_test.py#L444-L476
yield self.buy(tag='foo')
assert self.trades[-1].tag == 'foo'
or anything better you can think of.
@qacollective Have you managed to test it all? Does it work? :smiley:
Yes, I have indeed tested this change and it works for what I intended it to - you can now track particular trades through whatever tag you assign to it - totally solved the problem I was trying to fix!
@qacollective Care to add a simple unit test that confirms the working of the implementation? :smiley:
Wish I read the PR change before I implemented something similar. Nice :+1:
Why is this still open? Will it be merged? I will now have to manually search my trades and the only distinguishing feature they have is a unique stop loss. Trades are all executed at same time and price (but even using that would be a cludge). Please merge or inform me of how one might reference trades from orders.
@kernc why this pr not merged yet? it's really useful when add some custom extra data for tracking.
The PR is still missing:
- [x] tag entry in the Trades dataframe https://github.com/kernc/backtesting.py/blob/94d20da85e278102a0cc71b27a3a35b815e11648/backtesting/_stats.py#L57-L69
- [x] unit tests.
I guess I should get round to it. :sweat_smile:
It took a while, but now it's in. Thanks @qacollective for doing most of the legwork!
Hello,
just started to work with backtesting.py.
Working with the tag I just found that when it is not numerical an error is raised when printing accessing Strategy.orders. From the error message and looking into the code I found that this section appears to cause it. Line 408 on commit 0ce24d8
def __repr__(self):
return '<Order {}>'.format(', '.join(f'{param}={round(value, 5)}'
for param, value in (
('size', self.__size),
('limit', self.__limit_price),
('stop', self.__stop_price),
('sl', self.__sl_price),
('tp', self.__tp_price),
('contingent', self.is_contingent),
('tag', self.__tag),
) if value is not None))
since the tag is processed through the round function it will throw errors when providing a string tag. Unfortunately my python and git skills are not good enough to build and provide a fix. I still hope this post helps you to fix this eventually.