这是indexloc提供的服务,不要输入任何密码
Skip to content

Fixed incorrect data display using stocks/fa/est command #4940

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

Merged
merged 7 commits into from
May 1, 2023
Merged

Fixed incorrect data display using stocks/fa/est command #4940

merged 7 commits into from
May 1, 2023

Conversation

dot-agi
Copy link
Contributor

@dot-agi dot-agi commented May 1, 2023

Fixes #4938.

get_estimates has been revamped from the ground up to provide correct data for displaying on the terminal.

@reviewpad reviewpad bot added the feat S label May 1, 2023
@dot-agi dot-agi changed the title Hotfix/4938 Fixed incorrect data display using stocks\fa\est command May 1, 2023
@dot-agi dot-agi changed the title Fixed incorrect data display using stocks\fa\est command Fixed incorrect data display using stocks/fa/est command May 1, 2023
@dot-agi
Copy link
Contributor Author

dot-agi commented May 1, 2023

@jmaslek Is the test failing due to some expected values in test_estimates.txt?

@jmaslek
Copy link
Contributor

jmaslek commented May 1, 2023

@jmaslek Is the test failing due to some expected values in test_estimates.txt?

Yup! Since you changed the output slightly, the test needs to be re-recorded to capture what is expected. You should be able to do so with:

(do for both _model and _view)

pytest tests/openbb_terminal/stocks/fundamental_analysis/test_business_insider_model.py --rewrite-expected

@dot-agi
Copy link
Contributor Author

dot-agi commented May 1, 2023

@jmaslek Is the test failing due to some expected values in test_estimates.txt?

Yup! Since you changed the output slightly, the test needs to be re-recorded to capture what is expected. You should be able to do so with:

(do for both _model and _view)

pytest tests/openbb_terminal/stocks/fundamental_analysis/test_business_insider_model.py --rewrite-expected

I get the following error when I run this command - Exit: Uninstall brotli and brotlipy before running tests

How to solve this issue?

@jmaslek
Copy link
Contributor

jmaslek commented May 1, 2023

@jmaslek Is the test failing due to some expected values in test_estimates.txt?

Yup! Since you changed the output slightly, the test needs to be re-recorded to capture what is expected. You should be able to do so with:
(do for both _model and _view)
pytest tests/openbb_terminal/stocks/fundamental_analysis/test_business_insider_model.py --rewrite-expected

I get the following error when I run this command - Exit: Uninstall brotli and brotlipy before running tests

How to solve this issue?

pip uninstall Brotli -y

@dot-agi
Copy link
Contributor Author

dot-agi commented May 1, 2023

@jmaslek Is the test failing due to some expected values in test_estimates.txt?

Yup! Since you changed the output slightly, the test needs to be re-recorded to capture what is expected. You should be able to do so with:
(do for both _model and _view)
pytest tests/openbb_terminal/stocks/fundamental_analysis/test_business_insider_model.py --rewrite-expected

I get the following error when I run this command - Exit: Uninstall brotli and brotlipy before running tests
How to solve this issue?

pip uninstall Brotli -y

This worked, thanks for the help!

I have a question - what is the purpose of brotli and brotlipy package in OpenBB?

@jmaslek
Copy link
Contributor

jmaslek commented May 1, 2023

@jmaslek Is the test failing due to some expected values in test_estimates.txt?

Yup! Since you changed the output slightly, the test needs to be re-recorded to capture what is expected. You should be able to do so with:
(do for both _model and _view)
pytest tests/openbb_terminal/stocks/fundamental_analysis/test_business_insider_model.py --rewrite-expected

I get the following error when I run this command - Exit: Uninstall brotli and brotlipy before running tests
How to solve this issue?

pip uninstall Brotli -y

This worked, thanks for the help!

I have a question - what is the purpose of brotli and brotlipy package in OpenBB?

So Brotli is a compression algorithm (I believe its googles). IN our early days, we had issues with using pytest when the brotli compression was used. So it was not in the dependency tree. It was recently brought in with the video transcription, so in order to avoid issues with tests, we have a check to uninstall it

@codecov
Copy link

codecov bot commented May 1, 2023

Codecov Report

Patch coverage: 44.37% and project coverage change: -0.20 ⚠️

Comparison is base (df9a154) 58.29% compared to head (6ab0f77) 58.09%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4940      +/-   ##
===========================================
- Coverage    58.29%   58.09%   -0.20%     
===========================================
  Files          588      588              
  Lines        53666    53680      +14     
===========================================
- Hits         31283    31185      -98     
- Misses       22383    22495     +112     
Impacted Files Coverage Δ
...inal/core/sdk/controllers/crypto_sdk_controller.py 0.00% <ø> (ø)
...inal/core/sdk/controllers/stocks_sdk_controller.py 0.00% <ø> (ø)
...penbb_terminal/core/sdk/models/crypto_sdk_model.py 0.00% <0.00%> (ø)
...bb_terminal/core/sdk/models/portfolio_sdk_model.py 0.00% <0.00%> (ø)
...penbb_terminal/core/sdk/models/stocks_sdk_model.py 0.00% <ø> (ø)
openbb_terminal/core/sdk/sdk_helpers.py 36.92% <ø> (ø)
openbb_terminal/core/sdk/sdk_init.py 92.75% <ø> (ø)
openbb_terminal/core/sdk/trailmap.py 92.68% <ø> (ø)
openbb_terminal/dashboards/stream/Forecasting.py 0.00% <0.00%> (ø)
...bb_terminal/dashboards/stream/streamlit_helpers.py 0.00% <0.00%> (ø)
... and 19 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jmaslek jmaslek added this pull request to the merge queue May 1, 2023
Merged via the queue into OpenBB-finance:develop with commit 3d7b934 May 1, 2023
@dot-agi dot-agi deleted the hotfix/4938 branch May 8, 2023 04:00
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.

[Bug] fa/est -t AAPL -e quarterearnings - data is not returning properly.
2 participants