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 CFD and Commodity support for Interactive Brokers #1604

Merged
merged 5 commits into from
Apr 23, 2024

Conversation

DracheShiki
Copy link
Contributor

@DracheShiki DracheShiki commented Apr 23, 2024

Pull Request

add CFD and Commodity support for Interactivebrokers;
fix the strict_symbology for parsing Instrument/contract of Interactivebrokers

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this change been tested?

Tested with "XAUUSD.IBCMDTY", "XAUUSD.IBCFD", "IBUS30.IBCFD", "TSLA.IBCFD", "EUD/USD.IBCFD" by live trading and historical data downloading.
a doc that "IBCFD" and "IBCMDTY" used as venue (mapping to "SMART") might be updated.
Although it works with no bugs as far as i tested it, the cfd.pyx and commodity.pyx may need to be refined.

Thanks @rsmb7z for advice.

@CLAassistant
Copy link

CLAassistant commented Apr 23, 2024

CLA assistant check
All committers have signed the CLA.

…symbology for parsing Instrument/contract of Interactivebrokers

Signed-off-by: D <[email protected]>
@cjdsellers cjdsellers changed the title add CFD and Commodity support for Interactivebrokers Add CFD and Commodity support for Interactivebrokers Apr 23, 2024
@cjdsellers cjdsellers changed the title Add CFD and Commodity support for Interactivebrokers Add CFD and Commodity support for Interactive Brokers Apr 23, 2024
@cjdsellers
Copy link
Member

Hi @DracheShiki

Thanks so much for this awesome contribution 🙏.

Looks like we just have pre-commit failing so far. You can run pre-commit with:

make pre-commit

or

pre-commit run --all-files

You can also run the following to install the git hook to ensure it always runs on local commits too:

pre-commit install

It looks like some auto-formatting from black will occur, then you can push another commit.

@cjdsellers cjdsellers requested a review from rsmb7z April 23, 2024 08:06
@rsmb7z
Copy link
Collaborator

rsmb7z commented Apr 23, 2024

LGTM.

@DracheShiki
Copy link
Contributor Author

Hi @DracheShiki

Thanks so much for this awesome contribution 🙏.

Looks like we just have pre-commit failing so far. You can run pre-commit with:

make pre-commit

or

pre-commit run --all-files

You can also run the following to install the git hook to ensure it always runs on local commits too:

pre-commit install

It looks like some auto-formatting from black will occur, then you can push another commit.

oh i just pushed the old one back-up without pre-commit.
I will push one with it. But for the function "ib_contract_to_instrument_id_simplified_symbology(contract: IBContract)" i can't reduce the complex...

@rsmb7z
Copy link
Collaborator

rsmb7z commented Apr 23, 2024

I will push one with it. But for the function "ib_contract_to_instrument_id_simplified_symbology(contract: IBContract)" i can't reduce the complex...

For ib_contract_to_instrument_id_simplified_symbology and instrument_id_to_ib_contract_simplified_symbology at the end of line add comment # noqa: C901.

Signed-off-by: DracheShiki <[email protected]>
@DracheShiki
Copy link
Contributor Author

I will push one with it. But for the function "ib_contract_to_instrument_id_simplified_symbology(contract: IBContract)" i can't reduce the complex...

For ib_contract_to_instrument_id_simplified_symbology and instrument_id_to_ib_contract_simplified_symbology at the end of line add comment # noqa: C901.

well, i just get confused for a while that everytime i added "#noqa: C901" then i excuted "pre-commit run --all-files", the "#noqa: C901" would get deleted by precommit...Anyway i pushed a version with "#noqa: C901" and without excuting "pre-commit run --all-files" again xD

@rsmb7z
Copy link
Collaborator

rsmb7z commented Apr 23, 2024

well, i just get confused for a while that everytime i added "#noqa: C901" then i excuted "pre-commit run --all-files", the "#noqa: C901" would get deleted by precommit...Anyway i pushed a version with "#noqa: C901" and without excuting "pre-commit run --all-files" again xD

The reason it gets deleted is because it expects on the first (def) line, even if its multi-line def.

@DracheShiki
Copy link
Contributor Author

well, i just get confused for a while that everytime i added "#noqa: C901" then i excuted "pre-commit run --all-files", the "#noqa: C901" would get deleted by precommit...Anyway i pushed a version with "#noqa: C901" and without excuting "pre-commit run --all-files" again xD

The reason it gets deleted is because it expects on the first (def) line, even if its multi-line def.

Oh thanks…That is also out of my cognition.

@cjdsellers cjdsellers merged commit b008fb3 into nautechsystems:develop Apr 23, 2024
9 checks passed
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.

4 participants