From 002bcb9e476291fd8d3ef4b7f26be196e423fdaa Mon Sep 17 00:00:00 2001 From: Catalin Toma Date: Fri, 7 Oct 2022 15:30:52 +0300 Subject: [PATCH] #2684 [CT] Improve stocks/sia error handling, abstract duplicate code (#2723) Co-authored-by: Colin Delahunty <72827203+colin99d@users.noreply.github.com> --- .../sia_controller.py | 168 +++++++----------- .../test_sia_controller.py | 141 ++++++++++++--- 2 files changed, 183 insertions(+), 126 deletions(-) diff --git a/openbb_terminal/stocks/sector_industry_analysis/sia_controller.py b/openbb_terminal/stocks/sector_industry_analysis/sia_controller.py index ee105dc975b..8a213d01707 100644 --- a/openbb_terminal/stocks/sector_industry_analysis/sia_controller.py +++ b/openbb_terminal/stocks/sector_industry_analysis/sia_controller.py @@ -182,55 +182,6 @@ class SectorIndustryAnalysisController(BaseController): self.tickers: List = list() self.currency: str = "" - if ticker: - data = yf.utils.get_json(f"https://finance.yahoo.com/quote/{ticker}") - - if "summaryProfile" in data: - self.country = data["summaryProfile"]["country"] - if self.country not in financedatabase_model.get_countries(): - similar_cmd = difflib.get_close_matches( - self.country, - financedatabase_model.get_countries(), - n=1, - cutoff=0.7, - ) - if similar_cmd: - self.country = similar_cmd[0] - self.sector = data["summaryProfile"]["sector"] - if self.sector not in financedatabase_model.get_sectors(): - similar_cmd = difflib.get_close_matches( - self.sector, - financedatabase_model.get_sectors(), - n=1, - cutoff=0.7, - ) - if similar_cmd: - self.sector = similar_cmd[0] - self.industry = data["summaryProfile"]["industry"] - if self.industry not in financedatabase_model.get_industries(): - similar_cmd = difflib.get_close_matches( - self.industry, - financedatabase_model.get_industries(), - n=1, - cutoff=0.7, - ) - if similar_cmd: - self.industry = similar_cmd[0] - if "price" in data: - mktcap = data["price"]["marketCap"] - if mktcap < 50_000_000: - self.mktcap = "Nano" - elif mktcap < 300_000_000: - self.mktcap = "Micro" - elif mktcap < 2_000_000_000: - self.mktcap = "Small" - elif mktcap > 200_000_000_000: - self.mktcap = "Mega" - elif mktcap > 10_000_000_000: - self.mktcap = "Large" - else: - self.mktcap = "Mid" - if session and obbff.USE_PROMPT_TOOLKIT: choices: dict = {c: {} for c in self.controller_choices} choices["mktcap"] = {c: None for c in self.mktcap_choices} @@ -243,7 +194,7 @@ class SectorIndustryAnalysisController(BaseController): self.choices = choices self.completer = NestedCompleter.from_nested_dict(choices) - self.update_runtime_choices() + self.__load() def update_runtime_choices(self): """Update runtime choices""" @@ -345,61 +296,80 @@ class SectorIndustryAnalysisController(BaseController): else: self.ticker = ns_parser.ticker.upper() - data = yf.utils.get_json( - f"https://finance.yahoo.com/quote/{self.ticker}" + self.__load() + + def __load(self): + if self.ticker: + data = yf.utils.get_json(f"https://finance.yahoo.com/quote/{self.ticker}") + + if "summaryProfile" not in data or data["summaryProfile"] is None: + raise Exception( + f"Failed to load {self.ticker} Summary Profile from Yahoo Finance" ) - if "summaryProfile" in data: - self.country = data["summaryProfile"]["country"] - if self.country not in financedatabase_model.get_countries(): - similar_cmd = difflib.get_close_matches( - self.country, - financedatabase_model.get_countries(), - n=1, - cutoff=0.7, - ) - if similar_cmd: - self.country = similar_cmd[0] + if not data["summaryProfile"]["country"]: + raise Exception( + f"Failed to load {self.ticker} Country from Yahoo Finance" + ) - self.sector = data["summaryProfile"]["sector"] - if self.sector not in financedatabase_model.get_sectors(): - similar_cmd = difflib.get_close_matches( - self.sector, - financedatabase_model.get_sectors(), - n=1, - cutoff=0.7, - ) - if similar_cmd: - self.sector = similar_cmd[0] + if not data["summaryProfile"]["sector"]: + raise Exception( + f"Failed to load {self.ticker} Sector from Yahoo Finance" + ) - self.industry = data["summaryProfile"]["industry"] - if self.industry not in financedatabase_model.get_industries(): - similar_cmd = difflib.get_close_matches( - self.industry, - financedatabase_model.get_industries(), - n=1, - cutoff=0.7, - ) - if similar_cmd: - self.industry = similar_cmd[0] + if not data["summaryProfile"]["industry"]: + raise Exception( + f"Failed to load {self.ticker} Industry from Yahoo Finance" + ) - if "price" in data: - mktcap = data["price"]["marketCap"] - if mktcap < 50_000_000: - self.mktcap = "Nano" - elif mktcap < 300_000_000: - self.mktcap = "Micro" - elif mktcap < 2_000_000_000: - self.mktcap = "Small" - elif mktcap > 200_000_000_000: - self.mktcap = "Mega" - elif mktcap > 10_000_000_000: - self.mktcap = "Large" - else: - self.mktcap = "Mid" + self.country = data["summaryProfile"]["country"] + if self.country not in financedatabase_model.get_countries(): + similar_cmd = difflib.get_close_matches( + self.country, + financedatabase_model.get_countries(), + n=1, + cutoff=0.7, + ) + if similar_cmd: + self.country = similar_cmd[0] + self.sector = data["summaryProfile"]["sector"] + if self.sector not in financedatabase_model.get_sectors(): + similar_cmd = difflib.get_close_matches( + self.sector, + financedatabase_model.get_sectors(), + n=1, + cutoff=0.7, + ) + if similar_cmd: + self.sector = similar_cmd[0] + self.industry = data["summaryProfile"]["industry"] + if self.industry not in financedatabase_model.get_industries(): + similar_cmd = difflib.get_close_matches( + self.industry, + financedatabase_model.get_industries(), + n=1, + cutoff=0.7, + ) + if similar_cmd: + self.industry = similar_cmd[0] - self.stocks_data = {} - self.update_runtime_choices() + if "price" in data: + mktcap = data["price"]["marketCap"] + if mktcap < 50_000_000: + self.mktcap = "Nano" + elif mktcap < 300_000_000: + self.mktcap = "Micro" + elif mktcap < 2_000_000_000: + self.mktcap = "Small" + elif mktcap > 200_000_000_000: + self.mktcap = "Mega" + elif mktcap > 10_000_000_000: + self.mktcap = "Large" + else: + self.mktcap = "Mid" + + self.stocks_data = {} + self.update_runtime_choices() @log_start_end(log=logger) def call_industry(self, other_args: List[str]): diff --git a/tests/openbb_terminal/stocks/sector_industry_analysis/test_sia_controller.py b/tests/openbb_terminal/stocks/sector_industry_analysis/test_sia_controller.py index 06306fb97bb..e5b71642402 100644 --- a/tests/openbb_terminal/stocks/sector_industry_analysis/test_sia_controller.py +++ b/tests/openbb_terminal/stocks/sector_industry_analysis/test_sia_controller.py @@ -39,6 +39,39 @@ DF_STOCK = pd.DataFrame.from_dict( }, orient="index", ) +VALID_SUMMARY_PROFILE = { + "country": "United States", + "industry": "Auto Manufacturers", + "sector": "Consumer Cyclical", +} + +MOCK_TUPLE_SHORT = (["MOCK_COUN"], ["MOCK_SEC"], ["MOCK_INDUS"]) +MOCK_TUPLE = (["MOCK_COUNTRY"], ["MOCK_SECTOR"], ["MOCK_INDUSTRY"]) +MOCK_TUPLE_NONE = (["MOCK_COUNTRY"], ["MOCK_SECTOR"], ["MOCK_INDUSTRY"], None) + +MOCK_SUMMARY_PROFILE = ( + { + "country": "MOCK_COUNTRY", + "sector": "MOCK_SECTOR", + "industry": "MOCK_INDUSTRY", + }, +) +MOCK_SUMMARY_PROFILE_SHORT = ( + { + "country": "MOCK_COUNTRY", + "sector": "MOCK_SECTOR", + "industry": "MOCK_INDUSTRY", + }, +) +MOCK_SUMMARY_PROFILE_MISSING_COUNTRY = ( + {"country": "", "sector": "MOCK_SECTOR", "industry": "MOCK_INDUSTRY"}, +) +MOCK_SUMMARY_PROFILE_MISSING_INDUSTRY = ( + {"country": "MOCK_COUNTRY", "sector": "MOCK_SECTOR", "industry": ""}, +) +MOCK_SUMMARY_PROFILE_MISSING_SECTOR = ( + {"country": "MOCK_COUNTRY", "sector": "", "industry": "MOCK_INDUSTRY"}, +) @pytest.fixture(scope="module") @@ -509,13 +542,19 @@ def test_call_func_no_parser(func, mocker): @pytest.mark.vcr(record_mode="none") @pytest.mark.parametrize( - "countries, sectors, industries", + "countries, sectors, industries, summaryProfile", [ - (["MOCK_COUNTRY"], ["MOCK_SECTOR"], ["MOCK_INDUSTRY"]), - (["MOCK_COUN"], ["MOCK_SEC"], ["MOCK_INDUS"]), + MOCK_TUPLE + MOCK_SUMMARY_PROFILE, + MOCK_TUPLE_SHORT + MOCK_SUMMARY_PROFILE_SHORT, + MOCK_TUPLE_NONE, + MOCK_TUPLE + MOCK_SUMMARY_PROFILE_MISSING_COUNTRY, + MOCK_TUPLE + MOCK_SUMMARY_PROFILE_MISSING_INDUSTRY, + MOCK_TUPLE + MOCK_SUMMARY_PROFILE_MISSING_SECTOR, ], ) -def test_controller_init_summary_profile(countries, industries, mocker, sectors): +def test_controller_init_summary_profile( + countries, industries, mocker, sectors, summaryProfile +): path_controller = "openbb_terminal.stocks.sector_industry_analysis.sia_controller" # MOCK OBBFF @@ -530,13 +569,7 @@ def test_controller_init_summary_profile(countries, industries, mocker, sectors) ) # MOCK GET_JSON - mock_get_json = { - "summaryProfile": { - "country": "MOCK_COUNTRY", - "sector": "MOCK_SECTOR", - "industry": "MOCK_INDUSTRY", - }, - } + mock_get_json = {"summaryProfile": summaryProfile} target = f"{path_controller}.financedatabase_model.yf.utils.get_json" mocker.patch(target=target, return_value=mock_get_json) @@ -552,7 +585,20 @@ def test_controller_init_summary_profile(countries, industries, mocker, sectors) target = f"{path_controller}.financedatabase_model.get_industries" mocker.patch(target=target, return_value=industries) - sia_controller.SectorIndustryAnalysisController(ticker="MOCK_TICKER", queue=None) + if ( + summaryProfile is None + or not summaryProfile["country"] + or not summaryProfile["sector"] + or not summaryProfile["industry"] + ): + with pytest.raises(Exception): + sia_controller.SectorIndustryAnalysisController( + ticker="MOCK_TICKER", queue=None + ) + else: + sia_controller.SectorIndustryAnalysisController( + ticker="MOCK_TICKER", queue=None + ) @pytest.mark.vcr(record_mode="none") @@ -565,11 +611,29 @@ def test_controller_init_summary_profile(countries, industries, mocker, sectors) ], ) def test_controller_init_market_cap(market_cap, mocker): + path_controller = "openbb_terminal.stocks.sector_industry_analysis.sia_controller" + # MOCK GET_JSON - mock_get_json = {"price": {"marketCap": market_cap}} - target = "openbb_terminal.stocks.sector_industry_analysis.financedatabase_model.yf.utils.get_json" + mock_get_json = { + "price": {"marketCap": market_cap}, + "summaryProfile": VALID_SUMMARY_PROFILE, + } + + target = f"{path_controller}.financedatabase_model.yf.utils.get_json" mocker.patch(target=target, return_value=mock_get_json) + # MOCK GET_COUNTRIES + target = f"{path_controller}.financedatabase_model.get_countries" + mocker.patch(target=target, return_value=[VALID_SUMMARY_PROFILE["country"]]) + + # MOCK GET_SECTORS + target = f"{path_controller}.financedatabase_model.get_sectors" + mocker.patch(target=target, return_value=[VALID_SUMMARY_PROFILE["sector"]]) + + # MOCK GET_INDUSTRIES + target = f"{path_controller}.financedatabase_model.get_industries" + mocker.patch(target=target, return_value=[VALID_SUMMARY_PROFILE["industry"]]) + sia_controller.SectorIndustryAnalysisController(ticker="MOCK_TICKER", queue=None) @@ -697,13 +761,17 @@ def test_custom_reset(expected, ticker): @pytest.mark.vcr(record_mode="none") @pytest.mark.parametrize( - "countries, sectors, industries", + "countries, sectors, industries, summaryProfile", [ - (["MOCK_COUNTRY"], ["MOCK_SECTOR"], ["MOCK_INDUSTRY"]), - (["MOCK_COUN"], ["MOCK_SEC"], ["MOCK_INDUS"]), + MOCK_TUPLE + MOCK_SUMMARY_PROFILE, + MOCK_TUPLE_SHORT + MOCK_SUMMARY_PROFILE_SHORT, + MOCK_TUPLE_NONE, + MOCK_TUPLE + MOCK_SUMMARY_PROFILE_MISSING_COUNTRY, + MOCK_TUPLE + MOCK_SUMMARY_PROFILE_MISSING_INDUSTRY, + MOCK_TUPLE + MOCK_SUMMARY_PROFILE_MISSING_SECTOR, ], ) -def test_call_load(countries, industries, mocker, sectors): +def test_call_load(countries, industries, mocker, sectors, summaryProfile): path_controller = "openbb_terminal.stocks.sector_industry_analysis.sia_controller" # MOCK LOAD @@ -711,13 +779,7 @@ def test_call_load(countries, industries, mocker, sectors): mocker.patch(target=target, return_value=DF_STOCK) # MOCK GET_JSON - mock_get_json = { - "summaryProfile": { - "country": "MOCK_COUNTRY", - "sector": "MOCK_SECTOR", - "industry": "MOCK_INDUSTRY", - }, - } + mock_get_json = {"summaryProfile": summaryProfile} target = f"{path_controller}.financedatabase_model.yf.utils.get_json" mocker.patch(target=target, return_value=mock_get_json) @@ -747,7 +809,17 @@ def test_call_load(countries, industries, mocker, sectors): "--start=2021-12-17", "--end=2021-12-18", ] - controller.call_load(other_args=other_args) + + if ( + summaryProfile is None + or not summaryProfile["country"] + or not summaryProfile["sector"] + or not summaryProfile["industry"] + ): + with pytest.raises(Exception): + controller.call_load(other_args=other_args) + else: + controller.call_load(other_args=other_args) @pytest.mark.vcr(record_mode="none") @@ -767,10 +839,25 @@ def test_call_load_market_cap(market_cap, mocker): mocker.patch(target=target, return_value=DF_STOCK) # MOCK GET_JSON - mock_get_json = {"price": {"marketCap": market_cap}} + mock_get_json = { + "price": {"marketCap": market_cap}, + "summaryProfile": VALID_SUMMARY_PROFILE, + } target = f"{path_controller}.financedatabase_model.yf.utils.get_json" mocker.patch(target=target, return_value=mock_get_json) + # MOCK GET_COUNTRIES + target = f"{path_controller}.financedatabase_model.get_countries" + mocker.patch(target=target, return_value=[VALID_SUMMARY_PROFILE["country"]]) + + # MOCK GET_SECTORS + target = f"{path_controller}.financedatabase_model.get_sectors" + mocker.patch(target=target, return_value=[VALID_SUMMARY_PROFILE["sector"]]) + + # MOCK GET_INDUSTRIES + target = f"{path_controller}.financedatabase_model.get_industries" + mocker.patch(target=target, return_value=[VALID_SUMMARY_PROFILE["industry"]]) + # MOCK UPDATE_RUNTIME_CHOICES mocker.patch( target=f"{path_controller}.SectorIndustryAnalysisController.update_runtime_choices",