-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Replace C2 symbol for C2 Exchange symbol #8604
Open
Marinovsky
wants to merge
13
commits into
QuantConnect:master
Choose a base branch
from
Marinovsky:bug-8554-ReplaceC2SymbolForExchangeSymbol
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Replace C2 symbol for C2 Exchange symbol #8604
Marinovsky
wants to merge
13
commits into
QuantConnect:master
from
Marinovsky:bug-8554-ReplaceC2SymbolForExchangeSymbol
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Martin-Molinero
approved these changes
Feb 21, 2025
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.
Thank you, looks good, minor comments shared 👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
C2 recommends using
ExchangeSymbol
instead ofC2Symbol
, thus this PR aims to replace LEANC2Symbol
toC2ExchangeSymbol
. The payload implemented is the one described in their API docs: https://api-docs.collective2.com/apis/general/swagger/strategies/c2_api_strategies/setdesiredpositions_post . It's worth saying, not all markets present in LEAN had a MIC exchange code, according to the following list: https://www.iso20022.org/sites/default/files/ISO10383_MIC/ISO10383_MIC.pdf . Below, is the list of LEAN markets for which no MIC code was found:XLIF - EURONEXT LIFFE - Not present in LEAN
OSE - LEAN only supports index, no future
FTX - LEAN only supports crypto, no future
InteractiveBrokers - LEAN only supports CFD, no future
XNYB - Not even present in ISO webpage
XASX - ASX- All markets - Not presetn in LEAN
XKBT - KANSAS CITY BOARD OF TRADE - Not present in LEAN
OANDA - Not even present in the ISO webpage
dukascopy - Not even present in the ISO webpage
ice - used the MIC code for Intercontinental Exchange
CBOE- used the MIC code for CBOE GLOBAL MARKETS INC.
bitfinex - Not even presetn in the ISO webpage
BITHUMB - Not even present in the ISO webpage
Binance- Not even presetn in the ISO webpage
poloniex - Not even present in the ISO webpage
coinone - Not even present in the ISO webpage
hitbtc - Not even present in the ISO webpage
bittrex - Not even present in the ISO webpage
binanceus - Not even present in the ISO webpage
bybit - Not even present in the ISO webpage
Coinbase - LEAN only supports crypto, no derivatives
Kraken - LEAN only supports crypto, no derivatives
Bitstamp - LEAN only supports crypto, no derivatives
okcoin - Not even present in the ISO webpage
Besides that, since the GH issue #8577 was related to this one, I addressed it on this same PR.
Related Issue
Closes #8554, #8577
Motivation and Context
With this change, the signals sent to Collective2 will follow their suggested guidelines
Requires Documentation Change
N/A
How Has This Been Tested?
I updated the unit tests
SignalExportTargetTests.SendsTargetsToCollective2Appropiately
to assert the message sent, in each of them, is the expected one across different scenarios. I also used to regression testCollective2SignalExportDemonstrationAlgorithm
to assert the message sent was accepted by Collective2 API.Types of changes
Checklist:
bug-<issue#>-<description>
orfeature-<issue#>-<description>