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

Adds crypto fees revenue command #4067

Merged
merged 34 commits into from
Jun 13, 2023
Merged

Adds crypto fees revenue command #4067

merged 34 commits into from
Jun 13, 2023

Conversation

jose-donato
Copy link
Contributor

Description

Integrates crypto fees revenue adapter from @CryptoStats_. There are many other adapters (FREE) that we can integrate from cryptostats

Still need to:

  • add tests
  • add to sdk

How has this been tested?

  • Please describe the tests that you ran to verify your changes.
  • Provide instructions so we can reproduce.
  • Please also list any relevant details for your test configuration.
  • Make sure affected commands still run in terminal
  • Ensure the SDK still works
  • Check any related reports

Checklist:

Others

  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.

@reviewpad reviewpad bot added the feat S Small T-Shirt size Feature label Jan 29, 2023
@jose-donato jose-donato changed the title fix: initial commit Adds crypto fees revenue command Jan 29, 2023
@jose-donato jose-donato requested a review from jmaslek March 29, 2023 10:40
@reviewpad reviewpad bot added feat XL Extra Large feature and removed feat S Small T-Shirt size Feature labels Mar 29, 2023
raise Exception(f"Status code: {response.status_code}. Reason: {response.text}")
try:
df = pd.DataFrame(response.json())
df.replace({float(np.nan): None}, inplace=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

inplace=True is bad, You should also use fillna

Comment on lines 64 to 69
export_data(
export,
os.path.dirname(os.path.abspath(__file__)),
"fees",
df,
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

sheet name argument

)

parser.add_argument(
"-mc",
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should not have 2 letters on single dash argument

help="Include the market cap rank",
)
parser.add_argument(
"-tvl",
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

@codecov
Copy link

codecov bot commented Apr 11, 2023

Codecov Report

Patch coverage: 55.25% and project coverage change: -0.24 ⚠️

Comparison is base (df9a154) 58.29% compared to head (614c001) 58.06%.

❗ Current head 614c001 differs from pull request most recent head 6e01e45. Consider uploading reports for the commit 6e01e45 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4067      +/-   ##
===========================================
- Coverage    58.29%   58.06%   -0.24%     
===========================================
  Files          588      590       +2     
  Lines        53666    53921     +255     
===========================================
+ Hits         31283    31307      +24     
- Misses       22383    22614     +231     
Impacted Files Coverage Δ
.../core/plots/plotly_ta/plugins/volatility_plugin.py 31.37% <0.00%> (-1.29%) ⬇️
...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% <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%> (ø)
... and 37 more

... and 3 files with indirect coverage changes

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

Copy link
Collaborator

@jmaslek jmaslek left a comment

Choose a reason for hiding this comment

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

fix the source and then its picasso level beauty

@jose-donato
Copy link
Contributor Author

jose-donato commented May 3, 2023

@jmaslek fixed the tests related to this pr
failing on a stocks on ERROR tests/openbb_terminal/stocks/fundamental_analysis/test_yahoo_finance_view.py::test_display_splits[display_splits-get_splits]

@jmaslek jmaslek added this pull request to the merge queue Jun 13, 2023
Merged via the queue into develop with commit c5d5b51 Jun 13, 2023
@piiq piiq deleted the feature/cryptostats_fees branch July 12, 2023 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat XL Extra Large feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants