-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Crypto api fixes #1959
Crypto api fixes #1959
Conversation
Makes pa actice and adds country to the df. The groupby command also gets percents of holding allocation. It also fixes warnings and prepares for a later pr that I'm currently working on.
black linter
…minal into Crypto-API-fixes
… into Crypto-API-fixes
Co-Authored-By: Chavithra <6114398+Chavithra@users.noreply.github.com>
…kTerminal into Crypto-API-fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@northern-64bit about the market command, that was specific to the coin loaded, e.g. ETH. I tried out other coins and that works well.
We could replace the value with N/A to reflect the absence of data.
@northern-64bit Tested and working well! Very nice job on this :) A minor thing - can we remove the long URL, and keep the short one that you have? It's very looong! 😅 |
then let's get it merged! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LEGOOO 🚀
Description
Fixes in the crypto menu for the api. This involves giving many view functions default values and changing some and adding doc strings.
How has this been tested?
These changes needs to be tested since I don't have the required api keys in order to test the changes. Some tests fail and I don't have the knowledge to fix them so help would be appreciated.
Checklist:
Others
pre-commit install
.pytest tests/...
.