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

Fix crypto/defi/vaults not showing for default and -k lp #1957

Merged
merged 3 commits into from
Jun 17, 2022
Merged

Conversation

montezdesousa
Copy link
Contributor

Description

Solving issue #1940 where vaults were not being displayed. Error occurred because 'NaN' values on the APY field are being replaced by "NA" before the sort_values method acted on it, so it was trying to compare str to float. Just changed the replace of 'NaN' values to after the sort_values method.

Before:
image

After:
image
image

  • Summary of the change / bug fix.
  • Link # issue, if applicable.
  • Screenshot of the feature or the bug before/after fix, if applicable.
  • Relevant motivation and context.
  • List any dependencies that are required for this change.

How has this been tested?

pytest tests/openbb_terminal/cryptocurrency/defi/test_coindix_model.py --record-mode=rewrite
pytest tests/openbb_terminal/cryptocurrency/defi/test_coindix_view.py --record-mode=rewrite

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/....

@montezdesousa montezdesousa added the bug Fix bug label Jun 17, 2022
@montezdesousa montezdesousa self-assigned this Jun 17, 2022
@montezdesousa montezdesousa linked an issue Jun 17, 2022 that may be closed by this pull request
@colin99d colin99d self-requested a review June 17, 2022 12:52
Copy link
Contributor

@colin99d colin99d left a comment

Choose a reason for hiding this comment

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

Looks great! Nice work.

@montezdesousa
Copy link
Contributor Author

Looks great! Nice work.

Thanks!!

@colin99d colin99d merged commit 99dce24 into main Jun 17, 2022
@colin99d colin99d deleted the issue1940 branch June 17, 2022 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fix bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] crypto/defi/vaults not working
2 participants