-
-
Notifications
You must be signed in to change notification settings - Fork 46
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
Complex orders #107
Complex orders #107
Conversation
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.
This is great, thanks for the contribution! A couple small comments. Also, if you could add a test for this, that would be great!
@@ -269,16 +295,10 @@ class ComplexOrder(TastytradeJsonDataclass): | |||
""" | |||
Dataclass containing information about a complex order. | |||
""" | |||
id: str |
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.
I believe these lines are needed for previously placed complex orders, you can confirm by calling Account.get_history
after placing a complex order. If so, this could be renamed to PlacedComplexOrder
to maintain the naming convention.
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.
These lines are no longer in the return value of placing complex orders. If there are other complex orders than OTOCO and OCO, then maybe, but those 2 are the only one I found.
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.
Check the value after calling Account.get_history
with a complex order in the history, that's why this is there in the first place
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.
Looks like it's for pairs trades!
tastytrade/utils.py
Outdated
@@ -45,3 +46,11 @@ def validate_response(response: Response) -> None: # pragma: no cover | |||
error_message += f"\n{error['code']}: {error['message']}" | |||
|
|||
raise TastytradeError(error_message) | |||
|
|||
|
|||
def object_to_dict(obj): |
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.
Maybe I'm missing where you're using this but I didn't see any usages? Also it may be repeating the functionality of pydantic's BaseModel.to_dict
.
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.
It's a ghost I forgot about. Will roll back the change.
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.
I tested with both get_history and get_order_history and made a $0.10 profit in the process, as the complex order doesn't work in the certification environment.
In get_history it is absolutely not visible that the transactions once were part of a complex order.
In get_order_history it is visible, but there is no "dictionary" complex_order, just id's, which are attributes of the class.
* streamers use async context managers; prod session in testing * fix lint * Update CONTRIBUTING.md * no tests on main branch * update tests
* Make market metrics optional * Enable manual workflow triggering
Please note you can run tests locally with |
@Graeme22 The tests now run locally, but 95% is not reached. I think this is mainly because streamer.py has a low coverage, but that was not touched. |
It's because you added new code, but no tests for it! If you're not comfortable writing tests, lmk and I'll do it. |
It seems impossible to test the complex orders.
|
I was able to test successfully in a certification account! |
Thanks for this, I went ahead and merged! For a OCO order, you create a Appreciate the contributions! |
You're most welcome. Always fun to contribute to this kind of projects. |
Hi Graeme, i was trying several times using the certification account class. I was not able to place a simpel order. Could you pls provide a code snippet for this. My API Sandbox account is up and running. |
Is the code example in #63 not working for you? |
The general usage of the certification class. |
Just make a certification session, here's the docs on that: https://tastyworks-api.readthedocs.io/en/latest/sessions.html |
Its working as expected. I was running into an Issue because I never got filled due to using a realistic! market price on a symbol. Sandbox Environment |
Description
Add the possibility to create complex orders:
Also cancel channel has been added to the DXLinkStreamer
Related issue(s)
Fixes: #63
Pre-merge checklist
Please note that, in order to pass the tests, you'll need to set up your Tastytrade credentials as repository secrets on your local fork. Read more at CONTRIBUTING.md.