-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[Feature] OptionsChains Properties #6564
Conversation
…minal into feature/options-chains-properties
For readability purposes and general best practices, I think it makes sense to define all the functions outside of the model file. i.e for straddle, move the logic goes into a seperate util/helper somewhere and then call straddle(self.dataframe, ...) (Also - I would have thought this goes into an options specific extension, but no new deps are added, so is good) |
The purpose is to bind them to the class specifically, because this is so specific to this one particular piece of data, so that any validated instance of the class can use them. If this was doing something like incorporating a pricing or volatility model, I would agree that it should be a separate extension, but that is not within scope here. |
Yeah I have no argument for it not being in core. But for readability and organization, you should definitely break out the calculation logic from the model file. |
openbb_platform/providers/intrinio/tests/test_intrinio_fetchers.py
Outdated
Show resolved
Hide resolved
openbb_platform/providers/tradier/openbb_tradier/models/options_chains.py
Outdated
Show resolved
Hide resolved
openbb_platform/providers/tradier/tests/test_tradier_fetchers.py
Outdated
Show resolved
Hide resolved
openbb_platform/providers/yfinance/tests/test_yfinance_fetchers.py
Outdated
Show resolved
Hide resolved
…minal into feature/options-chains-properties
Thanks for moving to a separate file. I can confirm the following work (using chains("AAPL").results and chains("IONQ", provider="intrinio"):
Attributes are all there. From a code styling, looks like a lot has been repeated and could be cleaned up a little bit to make it easier to read. Nice work. |
* stashing * update providers * edge cases * linter * grammar police * yfinance tests? * linter * more linting * one test update * move properties code to separate file * to_df * tmx return type * tradier tests * cboe cassettes * elif instead of if * sort expirations and strikes * test cassettes * change tests --------- Co-authored-by: Igor Radovanovic <74266147+IgorWounds@users.noreply.github.com> Co-authored-by: James Maslek <jmaslek11@gmail.com>
* stashing * update providers * edge cases * linter * grammar police * yfinance tests? * linter * more linting * one test update * move properties code to separate file * to_df * tmx return type * tradier tests * cboe cassettes * elif instead of if * sort expirations and strikes * test cassettes * change tests --------- Co-authored-by: Igor Radovanovic <74266147+IgorWounds@users.noreply.github.com> Co-authored-by: James Maslek <jmaslek11@gmail.com>
* stashing * update providers * edge cases * linter * grammar police * yfinance tests? * linter * more linting * one test update * move properties code to separate file * to_df * tmx return type * tradier tests * cboe cassettes * elif instead of if * sort expirations and strikes * test cassettes * change tests --------- Co-authored-by: Igor Radovanovic <74266147+IgorWounds@users.noreply.github.com> Co-authored-by: James Maslek <jmaslek11@gmail.com>
WIP
More explanation to follow.
Why?:
What?:
data.results?
, from all providers. This contains a summary of all properties and methods (pasted below).Impact (1-2 sentences or a bullet point list):
derivatives.options.chains
returned as a Dictionary of arrays.to_df()
.Testing Done:
Reviewer Notes:
last_price
property can be set as a replacement. This can also be used as a manual override to the returned data of the provider.