-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[Interactive Brokers] Add Subscribe Index Price functionality #3514
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
base: develop
Are you sure you want to change the base?
[Interactive Brokers] Add Subscribe Index Price functionality #3514
Conversation
|
If only one value is produced would TradeTick be more appropriate? |
cjdsellers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Murph24
Thanks for the contribution!
Ideally we need some test coverage on this. In the past this was follow up work maintainers could take on, however with the recent increase in PRs, we have to insist on complete test coverage for new functionality.
One additional comment:
Tick data not cleared after creating update:
Compare to _try_create_quote_tick_from_market_data:
# Quote tick clears after use:
self._subscription_tick_data[req_id].clear()
But _try_create_index_price_tick_from_market_data doesn't clear. This means every process_tick_price callback will emit an IndexPriceUpdate even if
the price hasn't changed (the old value stays in tick_data[4]).
|
|
||
| if price is not None: | ||
| instrument_id = InstrumentId.from_str(subscription.name[0]) | ||
| instrument = self._cache.instrument(instrument_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check for None here, log an error and return (common pattern in other adapters). This is clearer than an attribute error on a NoneType later.
@faysou Can you clarify? Are you saying why not just subscribe to the index with: self.subscribe_trade_ticks(instrument_id=InstrumentId.from_str("^SPX.CBOE")) ? This does not work with the adapter as-is -or- are you suggesting to implement the SubscribeIndexPrice, but have it emit a TradeTick? That wouldn't seem correct to me, but you guys know this adapter much better than I do. |
Thanks for taking a look! Yes this would make sense, however I don't see this anywhere else in the adapter?
|
|
I don't know things much better than you, I'm just a contributor like you. I'm just saying that a quote is associated to a bid ask and last prices to trade ticks so it would be better to subscribe to trade ticks in this case and then there would maybe be nothing to change. Can you test if subscribing to trade ticks for the index works ? |
Pull Request
NautilusTrader prioritizes correctness and reliability, please follow existing patterns for validation and testing.
CONTRIBUTING.mdand followed the established practicesSummary
To subscribe to tick updates for index instruments such as SPX.CBOE, we need to call eclient.reqMktData. For an index we want the last price (tickType 4) and not the typical bid\ask tick types. Implementing the subscribe_index_prices functionality allows for a way to differentiate between typical bid\ask ticks and index price ticks in process_tick_price() of market_data.py
Related Issues/PRs
Addresses #3413
Type of change
Breaking change details (if applicable)
None
Documentation
docs/developer_guide/docs.md)Release notes
RELEASES.mdthat follows the existing conventions (when applicable)Testing
Possible this needs new test coverage?