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

[bug] Fix SettingWithCopyWarning and export error #2481

Merged
merged 6 commits into from
Sep 2, 2022

Conversation

boyestrous
Copy link
Contributor

@boyestrous boyestrous commented Sep 1, 2022

Description

  • Summary of the change / bug fix.
  1. Fixes SettingWithCopyWarning from Pandas in the stocks/options and stocks/ba modules\
  2. Also fixes error with stocks/ba --export flag (it was unable to export)
  3. Created unit tests for google_view.py and helper_funcs.py that would have caught the export flag issue
  4. Updated Docstrings for the impacted functions

How has this been tested?

  • Please describe the tests that you ran to verify your changes.
    Compared raw output from the google_model to the actual output. I left a detailed explanation of the cause of the SettingWithCopyWarning in the issue.

    TL;DR version - the warning only intermittently appears because of the varying data returned from google. Creating a copy of the original DF after filtering removes this warning.

  • Provide instructions so we can reproduce.

for changes in yfinance_model.py - unit tests already exist and continued to work

I tested the removal of the warning by running the following command before and after adding my changes
/stocks/options/chains

for changes to stocks/ba

I wrote a pytest called test_display_queries in test_google_view to compare the expected stdout. (I followed the Contributing guidelines for pytests visualizations)

I manually verified the output of the functions (prior to writing the pytest) with the following steps:

  1. open a python terminal

  2. import the following

    from openbb_terminal.common.behavioural_analysis import google_model as gm
    from openbb_terminal.common.behavioural_analysis import google_view as gv
    import pandas as pd
    
  3. get the response from google_model, save as gm_response
    >>> gm_response = gm.get_queries('TSLA')

  4. print the 'top' df from within gm_response
    >>> gm_response['TSLA']['top']

  5. run the following and verify that the top X values match the dict from the previous one
    gv.display_queries('TSLA')

  • Please also list any relevant details for your test configuration.

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

@boyestrous boyestrous changed the title Fix SettingWithCopyWarning and export error [bug] Fix SettingWithCopyWarning and export error Sep 1, 2022
@boyestrous boyestrous marked this pull request as ready for review September 1, 2022 22:26
@jmaslek jmaslek added the enhancement Enhancement label Sep 2, 2022
@jmaslek jmaslek merged commit db844a5 into OpenBB-finance:main Sep 2, 2022
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