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

Hotfix/options screener fix #4335

Merged
merged 62 commits into from
Feb 28, 2023
Merged

Hotfix/options screener fix #4335

merged 62 commits into from
Feb 28, 2023

Conversation

deeleeramone
Copy link
Contributor

@deeleeramone deeleeramone commented Feb 26, 2023

This patch is for #4250

Done:

  • Restores the options screener
  • Moves presets to Miscellaneous folder and updates the path
  • Removes stale presets, adds new sample presets.
  • Adjust columns being returned to be more intuitive.
  • fixes tests
  • adds options screener to SDK.

@deeleeramone deeleeramone added the bug Fix bug label Feb 26, 2023
@reviewpad reviewpad bot added the feat XL Extra Large feature label Feb 26, 2023
@jmaslek
Copy link
Collaborator

jmaslek commented Feb 28, 2023

sdk was not properly generated: AttributeError: 'TaRoot' object has no attribute 'hodges_tompkins'

You need to run python generate_sdk.py .

Doing this results in errors (AttributeError: module 'openbb_terminal.common.technical_analysis.volatility_model' has no attribute 'rvol_hodges_tompkins').

Theres also an extra trailing , in economy.cpi that needs to be taken out to get the above error.

@jmaslek
Copy link
Collaborator

jmaslek commented Feb 28, 2023

sdk was not properly generated: AttributeError: 'TaRoot' object has no attribute 'hodges_tompkins'

You need to run python generate_sdk.py .

Doing this results in errors (AttributeError: module 'openbb_terminal.common.technical_analysis.volatility_model' has no attribute 'rvol_hodges_tompkins').

Theres also an extra trailing , in economy.cpi that needs to be taken out to get the above error.

i gotchu

@deeleeramone
Copy link
Contributor Author

Now what's up with the tests? :(


MODELS = volatility_model.VOLATILITY_MODELS
MOCK_DATA = pd.read_csv(
"tests/openbb_terminal/stocks/technical_analysis/csv/test_volatility_model/test_cones_df.csv",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break on windows

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which part breaks on Windows? Does it need to be called, MOCK_STOCK_DF?

from test_ta_controller.py:

MOCK_STOCK_DF = pd.read_csv(
    "tests/openbb_terminal/stocks/technical_analysis/csv/test_ta_controller/stock_df.csv",
    index_col=0,
)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

windows doesnt read /. Im fixing

@jmaslek
Copy link
Collaborator

jmaslek commented Feb 28, 2023

Now what's up with the tests? :(

idk. looking. Also have no idea what you did to generate 200 test changes with newlines at the end

@jmaslek jmaslek added this pull request to the merge queue Feb 28, 2023
Merged via the queue into develop with commit 14e5b18 Feb 28, 2023
@Chavithra Chavithra deleted the hotfix/options-screener-fix branch March 1, 2023 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fix bug feat XL Extra Large feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] stocks/options/scr doesn't seem to work due to Yahoo Finance bugs
2 participants