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

Clarify values by re-ordering rows in enterprise value report #1908

Merged

Conversation

stkerr
Copy link
Contributor

@stkerr stkerr commented Jun 6, 2022

Description

This PR re-organizes the rows in the enterprise value report so that they are more logically presented. They previously were presented directly in the order as the data provider returned them, which wasn't great for human consumption. This meant that tickers were down at the bottom of the list and the values that sum to compute EV were not in any reasonable order and the EV itself was intermixed with other numbers, making it hard to identify.

This PR puts the stock ticker and price first, then it puts all the rows below it that sum to be the enterprise value, with the EV at the bottom.

image

The command is reached with /stocks/fa/fmp/enterprise while a ticker has been loaded.

How has this been tested?

I've tested the change locally and ran it several time with different tickers. The pre-commit tests all passed as well.

Checklist:

Others

  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • My code passes all the checks pylint, flake8, black, ... To speed up development you should run pre-commit install.
  • New and existing unit tests pass locally with my changes. You can test this locally using pytest tests/....

@stkerr stkerr changed the title Re-order columns in enterprise value report Clarify values by re-ordering rows in enterprise value report Jun 6, 2022
@stkerr stkerr force-pushed the stkerr-reorder-enterprise-value-report branch from fe5f7c7 to 80b83fe Compare June 6, 2022 12:19
@stkerr stkerr marked this pull request as ready for review June 6, 2022 12:19
@stkerr
Copy link
Contributor Author

stkerr commented Jun 6, 2022

@DidierRLopes @jmaslek can you review this PR?

I tried to put a label on it, as per the open a PR process, but I don't think I have access to that? Or does the branch name have to be feature/... to be able to add labels?

Thanks!

@jmaslek jmaslek added the enhancement Enhancement label Jun 6, 2022
@jmaslek
Copy link
Collaborator

jmaslek commented Jun 6, 2022

I tried to put a label on it, as per the open a PR process, but I don't think I have access to that? Or does the branch name have to be feature/... to be able to add labels?

Thanks!

Thanks again! I believe that the label has to be added by one of our team (we should probably reflect that in the documentation).

The small change is messing with the tests. You can overwrite the fmp tests by running the following then committing:

pytest tests/openbb_terminal/stocks/fundamental_analysis/financial_modeling_prep/test_fmp_view.py --rewrite-expected

@stkerr
Copy link
Contributor Author

stkerr commented Jun 6, 2022

@jmaslek

The small change is messing with the tests. You can overwrite the fmp tests by running the following then committing:

I ran that command & added the updated tests to the PR.

@stkerr stkerr force-pushed the stkerr-reorder-enterprise-value-report branch from 018d49c to a2f6218 Compare June 6, 2022 15:16
@stkerr stkerr force-pushed the stkerr-reorder-enterprise-value-report branch from a2f6218 to 766435e Compare June 6, 2022 17:00
@jmaslek jmaslek merged commit 610b19b into OpenBB-finance:main Jun 6, 2022
@stkerr stkerr deleted the stkerr-reorder-enterprise-value-report branch June 7, 2022 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants