Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Order.tag for tracking orders and trades #200

Merged
merged 7 commits into from
Dec 5, 2022

Conversation

qacollective
Copy link
Contributor

@qacollective qacollective commented Dec 17, 2020

Fixes #197
Fixes #53

Hello again. Many apologies for my butchering of the PR process. Git is certainly something I am no expert at. 😢

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.

@qacollective
Copy link
Contributor Author

@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.

@jjphung
Copy link

jjphung commented Feb 22, 2021

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?

@kernc
Copy link
Owner

kernc commented Feb 23, 2021

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:

def test_broker_hedging(self):
def coroutine(self):
yield self.buy(size=2)
assert len(self.trades) == 1
yield self.sell(size=1)
assert len(self.trades) == 2
self._Backtest(coroutine, hedging=True).run()
def test_broker_exclusive_orders(self):
def coroutine(self):
yield self.buy(size=2)
assert len(self.trades) == 1
yield self.sell(size=3)
assert len(self.trades) == 1
assert self.trades[0].size == -3
self._Backtest(coroutine, exclusive_orders=True).run()
def test_trade_multiple_close(self):
def coroutine(self):
yield self.buy()
assert self.trades
self.trades[-1].close(1)
self.trades[-1].close(.1)
yield
self._Backtest(coroutine).run()

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? 😃

@qacollective
Copy link
Contributor Author

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!

@kernc
Copy link
Owner

kernc commented Mar 14, 2021

@qacollective Care to add a simple unit test that confirms the working of the implementation? 😃

@crazy25000
Copy link
Contributor

Wish I read the PR change before I implemented something similar. Nice 👍

@p-baum
Copy link

p-baum commented Jun 2, 2021

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 kernc force-pushed the master branch 3 times, most recently from 7fd493d to e7981c7 Compare December 13, 2021 01:18
@guzuomuse
Copy link

@kernc
why this pr not merged yet? it's really useful when add some custom extra data for tracking.

@kernc
Copy link
Owner

kernc commented Jan 12, 2022

The PR is still missing:

  • tag entry in the Trades dataframe
    # Came straight from Backtest.run()
    trades_df = pd.DataFrame({
    'Size': [t.size for t in trades],
    'EntryBar': [t.entry_bar for t in trades],
    'ExitBar': [t.exit_bar for t in trades],
    'EntryPrice': [t.entry_price for t in trades],
    'ExitPrice': [t.exit_price for t in trades],
    'PnL': [t.pl for t in trades],
    'ReturnPct': [t.pl_pct for t in trades],
    'EntryTime': [t.entry_time for t in trades],
    'ExitTime': [t.exit_time for t in trades],
    })
    trades_df['Duration'] = trades_df['ExitTime'] - trades_df['EntryTime']
  • unit tests.

I guess I should get round to it. 😅

@kernc kernc force-pushed the master branch 9 times, most recently from 60eff81 to 109c352 Compare November 28, 2022 22:33
@kernc
Copy link
Owner

kernc commented Dec 5, 2022

It took a while, but now it's in. Thanks @qacollective for doing most of the legwork!

@kernc kernc changed the title Add tagging, object typing, fix pep8 Add order tagging Dec 5, 2022
@kernc kernc changed the title Add order tagging Add Order.tag for tracking orders and trades Dec 5, 2022
@kernc kernc merged commit 592ea41 into kernc:master Dec 5, 2022
@maneum1
Copy link

maneum1 commented Feb 27, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking id from order to trade Logging Orders
7 participants