-
Notifications
You must be signed in to change notification settings - Fork 4.2k
[Feature] Add Deribit Futures Data #6998
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
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.
Minor edits. This looks very cool!
openbb_platform/providers/deribit/openbb_deribit/models/futures_curve.py
Outdated
Show resolved
Hide resolved
@@ -107,7 +109,8 @@ def derivatives_futures_curve( # noqa: PLR0912 | |||
|
|||
provider = kwargs.get("provider", "") | |||
|
|||
df["expiration"] = df["expiration"].apply(to_datetime).dt.strftime("%b-%Y") | |||
if provider != "deribit": |
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.
Suggestion here:
Is there a way to avoid provider specific logic in the views? I see we have this in 3 views currently: this one, economy and fixedincome.
IIUC these are now used to "standardize" the data provided by providers to create a standardized chart. Would it be possible/make sense to move this logic to the provider level? For example to a custom model_dump
in the Data model which could add this remaining additional layer of standardization that is currently missing. This would make the views more universal, without hardcoded logic for providers which might or might not be there and generally would facilitate better modularity. Let me know what you think
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 like where your head is at, but I don't think this PR is the place to address this. We would need to reconsider the entire flow of the views.
For one, any presentation layer will always need a level of abstraction on top of the Data model. There aren't really many scenarios where dumping raw data into a chart is desirable.
If we were to move some logic to live in the provider, then we essentially need to create a chart/view inside every provider data model definition. This would create a lot of overhead and make assumptions for views that may not be installed or are not desired. Right now the charts are fully decoupled, but mixing it in the provider logic would blur that.
A custom "model_dump" doesn't necessarily help because the data needs to be aggregated as a complete table before some of downstream decisions can be made. A model_dump would only operate on single rows.
Additionally, there is logic applied that handles the user passing in data that is not directly from the raw data model output.
At the provider level, we don't have access to the OBBject and the ability to modify locals to pass forward to the charting extension.
That being said, the Platform charting ecosystem and framework needs to be revisited anyways - i.e, get rid of frontend-components - so, your points are very valid and should be a consideration when planning a modernized backend.
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.
Thanks for this thought out reply. Reading it was joy to my eyes and brain. ❤️
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've spotted that we have provider specific logic in the views and left a suggestion about it. Let me know what you think of it
Why?:
openbb-deribit
integration to include futures data.What?:
obb.derivatives.futures.instruments
obb.derivatives.futures.info
obb.derivatives.futures.curve
date
but provides,hours_ago
.obb.derivatives.futures.historical
Impact:
openbb-deribit
package is installed.Testing Done: