fix tokenomics error #2155

jaymgonzalez · 2022-07-24T11:23:30Z

Description

Summary of the change / bug fix.
When trying to get the tokenomics info for any assets I was faced with this error.

ETH not found on CoinGecko
Error: list indices must be integers or slices, not str

Error: cannot concatenate object of type '<class 'list'>'; only Series and DataFrame objs are valid

Error: not enough values to unpack (expected 2, got 0)

The first change I made was to force the symbol to lower case which made the not found on coingecko error disappear but the it complained about the ID.
Error: {'error': 'Could not find coin with the given id'}
Which was fixed by passing the coingecko ID to get_coin_tokenomics() function instead of the symbol.

This fixes made tokenomic info appear but there is still an issue when displaying the data in a graph, which I need more time to understand and investigate.

Link # issue, if applicable. -> Didn't create an issue, I just started to play and found the solution.
Screenshot of the feature or the bug before/after fix, if applicable.
Relevant motivation and context. -> I wanted the feature to work :)
List any dependencies that are required for this change. -> none

How has this been tested?

Please describe the tests that you ran to verify your changes.
Manual testing only
Provide instructions so we can reproduce.
crypto / load [ANY] / dd / tk
Please also list any relevant details for your test configuration.

Checklist:

Update our Hugo documentation following these guidelines. -> No need
Update our tests following these guidelines. -> No need
Make sure you are following our CONTRIBUTING guidelines.
If a feature was added make sure to add it to the corresponding scripts file.

Others

I have performed a self-review of my own code.
I have commented my code, particularly in hard-to-understand areas. -> No need
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/.... -> getting this error Uninstall brotli and brotlipy before running tests, not sure if i should uninstall the packages

jose-donato · 2022-07-24T11:25:59Z

ahah we just pushed exactly at the same time a fix to this error 😄

#2156

jaymgonzalez · 2022-07-24T11:27:47Z

@jose-donato oh really!? :D glad it's working, looking forward to find & fix more bugs :p

jose-donato

before fixing the linting/tests, can you just update this line on messari_view.py to merged_df['circulating_supply'] instead of merged_df['values'] ?

i will close my PR and we can work on yours. thanks 🚀

jaymgonzalez · 2022-07-24T11:58:21Z

Sure! I'll do it now!

…BTerminal into fix/tokenomicsDisplay

jose-donato

code looks good now. we just need to fix the tests/linting.

do you need any help on setting the pre-commit hooks for the linting?

to fix the tests you only need to run:
pytest tests/openbb_terminal/cryptocurrency/test_cryptocurrency_helpers.py --record-mode=rewrite

this will update the tests that are failing

jaymgonzalez · 2022-07-24T12:55:41Z

im getting this error, Exit: Uninstall brotli and brotlipy before running tests i'll uninstall them now and try again, please advise if those packages are needed and should be reinstalled, thanks

jaymgonzalez · 2022-07-24T13:00:03Z

@jose-donato im trying poetry remove brotli brotlipy but im getting a ValueError - Package brotli not found should i deactivate it? not sure what i need to do to make it work? have never used brotli before 😢

jaymgonzalez · 2022-07-24T13:10:49Z

also, i'd need help setting up the pre commit hooks for the linting...

DidierRLopes · 2022-07-24T14:13:28Z

Hey @jaymgonzalez ,

Can you do this in your environment pip list | grep brot that should show you if you have any brotli or brotlipy installed.

And then you can uninstall it with pip uninstall brotli or pip uninstall brotlipy.

This is necessary for generating the tests, so that there are no new tests created due to the installation of this package.

To set the pre-commit hook you can run pre-commit install. Hope this helps, let me know if you ned anything else!

jaymgonzalez · 2022-07-24T14:35:49Z

Hey @DidierRLopes, thanks for your comment, I installed pre-commit and uninstall the packages that were causing the issue. It let me run the test 🥳
I added them to my commit and will push them now, hopefully it works! 😊
Please let me know if there is anything else left to do after.

jose-donato · 2022-07-24T15:11:07Z

Hey @DidierRLopes, thanks for your comment, I installed pre-commit and uninstall the packages that were causing the issue. It let me run the test 🥳 I added them to my commit and will push them now, hopefully it works! 😊 Please let me know if there is anything else left to do after.

you need to run black openbb_terminal/cryptocurrency/due_diligence/messari_model.py to format that file that is failing. pre-commit didnt catch this because it happened in previous commits when you didnt have the hooks yet

…BTerminal into fix/tokenomicsDisplay

jaymgonzalez · 2022-07-24T21:42:17Z

hey @jose-donato, i run the command you posted in the comment before and it fixed Test / General Linting, thanks for that! unfortunately, test / base tests is not passing, there seems to be an issue with test_coin_chart() not sure how to test the test... 😮

jose-donato · 2022-07-24T21:46:55Z

hey @jose-donato, i run the command you posted in the comment before and it fixed Test / General Linting, thanks for that! unfortunately, test / base tests is not passing, there seems to be an issue with test_coin_chart() not sure how to test the test... 😮

Hey, thanks for your effort!
Did you try running this one?
pytest tests/openbb_terminal/cryptocurrency/test_cryptocurrency_helpers.py --record-mode=rewrite

jaymgonzalez · 2022-07-24T21:49:33Z

hey @jose-donato, i run the command you posted in the comment before and it fixed Test / General Linting, thanks for that! unfortunately, test / base tests is not passing, there seems to be an issue with test_coin_chart() not sure how to test the test... 😮

Hey, thanks for your effort! Did you try running this one? pytest tests/openbb_terminal/cryptocurrency/test_cryptocurrency_helpers.py --record-mode=rewrite

yes i did, i just re run it and try again! 😄
thank YOU guys! love the tool, glad im able to contribute a bit...

jose-donato · 2022-07-24T21:54:41Z

hey @jose-donato, i run the command you posted in the comment before and it fixed Test / General Linting, thanks for that! unfortunately, test / base tests is not passing, there seems to be an issue with test_coin_chart() not sure how to test the test... 😮

Hey, thanks for your effort! Did you try running this one? pytest tests/openbb_terminal/cryptocurrency/test_cryptocurrency_helpers.py --record-mode=rewrite

yes i did, i just re run it and try again! 😄 thank YOU guys! love the tool, glad im able to contribute a bit...

If it does not work I can try figure it out tomorrow.
Thank you for your contribution 🙌
Are you on our discord already?

…BTerminal into fix/tokenomicsDisplay

jaymgonzalez · 2022-07-24T22:15:00Z

hey @jose-donato, i run the command you posted in the comment before and it fixed Test / General Linting, thanks for that! unfortunately, test / base tests is not passing, there seems to be an issue with test_coin_chart() not sure how to test the test... 😮

Hey, thanks for your effort! Did you try running this one? pytest tests/openbb_terminal/cryptocurrency/test_cryptocurrency_helpers.py --record-mode=rewrite

yes i did, i just re run it and try again! 😄 thank YOU guys! love the tool, glad im able to contribute a bit...

If it does not work I can try figure it out tomorrow. Thank you for your contribution 🙌 Are you on our discord already?

yeah i tried again and it didnt work 🤷🏽‍♂️ if you can have a look tomorrow and let me know if there is anything else i can do to help 😄

just joined the discord! where do you usually you hang out in there?

DidierRLopes · 2022-07-24T22:40:45Z

hey @jose-donato, i run the command you posted in the comment before and it fixed Test / General Linting, thanks for that! unfortunately, test / base tests is not passing, there seems to be an issue with test_coin_chart() not sure how to test the test... 😮

Hey, thanks for your effort! Did you try running this one? pytest tests/openbb_terminal/cryptocurrency/test_cryptocurrency_helpers.py --record-mode=rewrite

yes i did, i just re run it and try again! 😄 thank YOU guys! love the tool, glad im able to contribute a bit...

If it does not work I can try figure it out tomorrow. Thank you for your contribution 🙌 Are you on our discord already?

yeah i tried again and it didnt work 🤷🏽‍♂️ if you can have a look tomorrow and let me know if there is anything else i can do to help 😄

just joined the discord! where do you usually you hang out in there?

Feel free to join us on any channel (perhaps #develop) and tag us: Didier & 1lluz10n

jaymgonzalez added 2 commits July 24, 2022 12:53

fix tokenomics error

027beb1

Merge branch 'main' into fix/tokenomicsDisplay

d2cd3ea

jose-donato self-requested a review July 24, 2022 11:25

jose-donato added the bug Fix bug label Jul 24, 2022

jose-donato requested changes Jul 24, 2022

View reviewed changes

jose-donato mentioned this pull request Jul 24, 2022

Fixes tk command from crypto due dilligence menu #2156

Closed

jaymgonzalez added 2 commits July 24, 2022 14:01

fix tokenomics graph

8b3756f

Merge branch 'fix/tokenomicsDisplay' of github.com:jaymgonzalez/OpenB…

76ea3cc

…BTerminal into fix/tokenomicsDisplay

jaymgonzalez requested a review from jose-donato July 24, 2022 12:05

jose-donato reviewed Jul 24, 2022

View reviewed changes

add tests

92483ff

jaymgonzalez requested a review from jose-donato July 24, 2022 14:36

Merge branch 'main' into fix/tokenomicsDisplay

2fe022d

jaymgonzalez and others added 3 commits July 24, 2022 23:15

add black command

adb7d37

Merge branch 'fix/tokenomicsDisplay' of github.com:jaymgonzalez/OpenB…

4e75302

…BTerminal into fix/tokenomicsDisplay

Merge branch 'main' into fix/tokenomicsDisplay

27b95b1

add test helpers

7a11598

jaymgonzalez added 2 commits July 24, 2022 23:54

Merge branch 'fix/tokenomicsDisplay' of github.com:jaymgonzalez/OpenB…

7c75cea

…BTerminal into fix/tokenomicsDisplay

add test helpers

9024667

jose-donato and others added 3 commits July 25, 2022 21:33

trying to fix tests

98ee586

trying to fix tests

2e8f7fa

Merge branch 'main' into fix/tokenomicsDisplay

65e76b3

jose-donato approved these changes Jul 25, 2022

View reviewed changes

jose-donato merged commit 9d6292c into OpenBB-finance:main Jul 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix tokenomics error #2155

fix tokenomics error #2155

jaymgonzalez commented Jul 24, 2022

jose-donato commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

jose-donato left a comment •

edited

Loading

jaymgonzalez commented Jul 24, 2022

jose-donato left a comment •

edited

Loading

jaymgonzalez commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

DidierRLopes commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

jose-donato commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

jose-donato commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

jose-donato commented Jul 24, 2022 •

edited

Loading

jaymgonzalez commented Jul 24, 2022

DidierRLopes commented Jul 24, 2022

fix tokenomics error #2155

fix tokenomics error #2155

Conversation

jaymgonzalez commented Jul 24, 2022

Description

How has this been tested?

Checklist:

Others

jose-donato commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

jose-donato left a comment • edited Loading

Choose a reason for hiding this comment

jaymgonzalez commented Jul 24, 2022

jose-donato left a comment • edited Loading

Choose a reason for hiding this comment

jaymgonzalez commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

DidierRLopes commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

jose-donato commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

jose-donato commented Jul 24, 2022

jaymgonzalez commented Jul 24, 2022

jose-donato commented Jul 24, 2022 • edited Loading

jaymgonzalez commented Jul 24, 2022

DidierRLopes commented Jul 24, 2022

jose-donato left a comment •

edited

Loading

jose-donato left a comment •

edited

Loading

jose-donato commented Jul 24, 2022 •

edited

Loading