From 7c7ba95a6e3fa41650966be3144b06f55c0c6790 Mon Sep 17 00:00:00 2001 From: Diogo Sousa Date: Mon, 30 Jan 2023 14:56:13 +0000 Subject: [PATCH 1/7] fix yf.info bugs and allow more bench options --- .../portfolio/attribution_model.py | 7 ++- .../portfolio/portfolio_controller.py | 17 +++++-- openbb_terminal/portfolio/portfolio_engine.py | 46 +++++++++++++++---- openbb_terminal/portfolio/portfolio_model.py | 18 ++++++-- 4 files changed, 71 insertions(+), 17 deletions(-) diff --git a/openbb_terminal/portfolio/attribution_model.py b/openbb_terminal/portfolio/attribution_model.py index dfa815b99ded..334b2f186528 100644 --- a/openbb_terminal/portfolio/attribution_model.py +++ b/openbb_terminal/portfolio/attribution_model.py @@ -9,6 +9,7 @@ import yfinance as yf import pandas as pd from openbb_terminal.decorators import log_start_end +from openbb_terminal.rich_config import console logger = logging.getLogger(__name__) @@ -68,7 +69,11 @@ def get_spy_sector_contributions( # Load in info sp500_tickers_data = get_daily_sector_prices(start_date, end_date) - weight_data = yf.Ticker(sectors_ticker).info["sectorWeightings"] + try: + weight_data = yf.Ticker(sectors_ticker).info["sectorWeightings"] + except Exception as _: # noqa + console.print("[red]Could not get info for S&P 500.") + return pd.DataFrame() # reformat Data weights: Dict[str, dict] = {"SPY": {}} diff --git a/openbb_terminal/portfolio/portfolio_controller.py b/openbb_terminal/portfolio/portfolio_controller.py index 25da7ba23af3..aa9cba7b2539 100644 --- a/openbb_terminal/portfolio/portfolio_controller.py +++ b/openbb_terminal/portfolio/portfolio_controller.py @@ -160,6 +160,10 @@ def __init__(self, queue: List[str] = None): self.update_choices() choices: dict = self.choices_default self.choices = choices + self.choices["bench"] = { + "--benchmark": {c: None for c in statics.BENCHMARK_CHOICES}, + "-b": "--benchmark", + } self.completer = NestedCompleter.from_nested_dict(choices) def update_choices(self): @@ -411,7 +415,6 @@ def call_bench(self, other_args: List[str]): dest="benchmark", required="-h" not in other_args, help="Set the benchmark for the portfolio. By default, this is SPDR S&P 500 ETF Trust (SPY).", - choices={c: {} for c in statics.BENCHMARK_CHOICES}, metavar="BENCHMARK", ) parser.add_argument( @@ -432,9 +435,11 @@ def call_bench(self, other_args: List[str]): "[red]Please first load transactions file using 'load'[/red]" ) else: - self.benchmark_name = statics.BENCHMARK_CHOICES.get(ns_parser.benchmark) - self.original_benchmark_ticker = ns_parser.benchmark - self.portfolio.set_benchmark(ns_parser.benchmark, ns_parser.full_shares) + if self.portfolio.set_benchmark(ns_parser.benchmark, ns_parser.full_shares): + self.benchmark_name = statics.BENCHMARK_CHOICES.get( + ns_parser.benchmark, ns_parser.benchmark + ) + self.original_benchmark_ticker = ns_parser.benchmark @log_start_end(log=logger) def call_alloc(self, other_args: List[str]): @@ -573,9 +578,13 @@ def call_attrib(self, other_args: List[str]): bench_result = attribution_model.get_spy_sector_contributions( start_date, end_date ) + if bench_result.empty: + return portfolio_result = attribution_model.get_portfolio_sector_contributions( start_date, self.portfolio.portfolio_trades ) + if portfolio_result.empty: + return # relative results - the proportions of return attribution if ns_parser.type == "relative": diff --git a/openbb_terminal/portfolio/portfolio_engine.py b/openbb_terminal/portfolio/portfolio_engine.py index 96b7b628d0a1..c845f6584aea 100644 --- a/openbb_terminal/portfolio/portfolio_engine.py +++ b/openbb_terminal/portfolio/portfolio_engine.py @@ -21,7 +21,8 @@ from openbb_terminal.terminal_helper import suppress_stdout # pylint: disable=E1136,W0201,R0902,C0302 -# pylint: disable=unsupported-assignment-operation,redefined-outer-name,too-many-public-methods, consider-using-f-string +# pylint: disable=unsupported-assignment-operation,redefined-outer-name +# pylint: too-many-public-methods, consider-using-f-string,disable=raise-missing-from logger = logging.getLogger(__name__) @@ -495,7 +496,7 @@ def __load_company_data(self): ] = info_list @log_start_end(log=logger) - def set_benchmark(self, symbol: str = "SPY", full_shares: bool = False): + def set_benchmark(self, symbol: str = "SPY", full_shares: bool = False) -> bool: """Load benchmark into portfolio. Parameters @@ -505,12 +506,15 @@ def set_benchmark(self, symbol: str = "SPY", full_shares: bool = False): full_shares: bool Whether to mimic the portfolio trades exactly (partial shares) or round down the quantity to the nearest number + + Returns + ------- + bool + True if successful, False otherwise """ p_bar = tqdm(range(4), desc=" Loading benchmark") - self.benchmark_ticker = symbol - self.benchmark_historical_prices = yf.download( symbol, start=self.inception_date - datetime.timedelta(days=1), @@ -519,6 +523,15 @@ def set_benchmark(self, symbol: str = "SPY", full_shares: bool = False): ignore_tz=True, )["Adj Close"] + if self.benchmark_historical_prices.empty: + console.print( + f"\n[red]Could not download benchmark data for {symbol}." + " Choose another symbol.\n[/red]" + ) + return False + + self.benchmark_ticker = symbol + p_bar.n += 1 p_bar.refresh() @@ -538,11 +551,6 @@ def set_benchmark(self, symbol: str = "SPY", full_shares: bool = False): p_bar.refresh() self.benchmark_returns = self.benchmark_historical_prices.pct_change().dropna() - self.benchmark_info = yf.Ticker(symbol).info - - p_bar.n += 1 - p_bar.refresh() - ( self.portfolio_returns, self.benchmark_returns, @@ -551,6 +559,20 @@ def set_benchmark(self, symbol: str = "SPY", full_shares: bool = False): p_bar.n += 1 p_bar.refresh() + try: + self.benchmark_info = yf.Ticker(symbol).info + except Exception as _: # noqa + console.print( + f"[red]\n\nCould not get info for {symbol}." + " This affects 'alloc' command.[/red]\n" + ) + return False + + p_bar.n += 1 + p_bar.refresh() + + return True + @log_start_end(log=logger) def __mimic_trades_for_benchmark(self, full_shares: bool = False): """Mimic trades from the transactions based on chosen benchmark assuming partial shares @@ -906,6 +928,12 @@ def calculate_allocation(self, category: str, recalculate: bool = False): Flag to force recalculate allocation if already exists """ + if not self.benchmark_info: + return + + if self.portfolio_trades.empty: + return + if category == "Asset": if ( self.benchmark_assets_allocation.empty diff --git a/openbb_terminal/portfolio/portfolio_model.py b/openbb_terminal/portfolio/portfolio_model.py index 6cd87b810b94..c9e8155f08e8 100644 --- a/openbb_terminal/portfolio/portfolio_model.py +++ b/openbb_terminal/portfolio/portfolio_model.py @@ -58,8 +58,8 @@ def generate_portfolio( transactions = PortfolioEngine.read_transactions(transactions_file_path) portfolio_engine = PortfolioEngine(transactions) portfolio_engine.generate_portfolio_data() - portfolio_engine.set_benchmark(symbol=benchmark_symbol, full_shares=full_shares) portfolio_engine.set_risk_free_rate(risk_free_rate) + portfolio_engine.set_benchmark(symbol=benchmark_symbol, full_shares=full_shares) return portfolio_engine @@ -92,7 +92,7 @@ def get_transactions(portfolio_engine: PortfolioEngine) -> pd.DataFrame: @log_start_end(log=logger) def set_benchmark( portfolio_engine: PortfolioEngine, symbol: str, full_shares: bool = False -): +) -> bool: """Load benchmark into portfolio Parameters @@ -106,6 +106,11 @@ def set_benchmark( Whether to mimic the portfolio trades exactly (partial shares) or round down the quantity to the nearest number + Returns + ------- + bool + True if successful, False otherwise + Examples -------- >>> from openbb_terminal.sdk import openbb @@ -113,7 +118,7 @@ def set_benchmark( >>> output = openbb.portfolio.bench(p, symbol="SPY") """ - portfolio_engine.set_benchmark(symbol=symbol, full_shares=full_shares) + return portfolio_engine.set_benchmark(symbol=symbol, full_shares=full_shares) @log_start_end(log=logger) @@ -430,6 +435,13 @@ def join_allocation( pd.DataFrame DataFrame with portfolio and benchmark allocations """ + + if portfolio.empty: + portfolio = pd.DataFrame(columns=[column, "Portfolio"]) + + if benchmark.empty: + benchmark = pd.DataFrame(columns=[column, "Benchmark"]) + combined = pd.merge(portfolio, benchmark, on=column, how="left") combined["Difference"] = combined["Portfolio"] - combined["Benchmark"] combined = combined.replace(np.nan, "-") From 469f11d39a8005147ebedb95891f1c8dad914a90 Mon Sep 17 00:00:00 2001 From: Diogo Sousa Date: Sun, 5 Feb 2023 12:07:28 +0000 Subject: [PATCH 2/7] leave false --- openbb_terminal/portfolio/portfolio_engine.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/openbb_terminal/portfolio/portfolio_engine.py b/openbb_terminal/portfolio/portfolio_engine.py index d683becef083..b52f639a5cce 100644 --- a/openbb_terminal/portfolio/portfolio_engine.py +++ b/openbb_terminal/portfolio/portfolio_engine.py @@ -227,7 +227,7 @@ def __preprocess_transactions(self): 13. Populate fields Sector, Industry and Country """ - p_bar = tqdm(range(14), desc="Preprocessing transactions") + p_bar = tqdm(range(14), desc="Preprocessing transactions", leave=False) try: # 0. If optional fields not in the transactions add missing @@ -513,7 +513,7 @@ def set_benchmark(self, symbol: str = "SPY", full_shares: bool = False) -> bool: True if successful, False otherwise """ - p_bar = tqdm(range(4), desc=" Loading benchmark") + p_bar = tqdm(range(4), desc=" Loading benchmark", leave=False) self.benchmark_historical_prices = yf.download( symbol, @@ -672,7 +672,9 @@ def __load_portfolio_historical_prices(self, use_close: bool = False): whether to use close or adjusted close prices """ - p_bar = tqdm(range(len(self.tickers)), desc=" Loading price data") + p_bar = tqdm( + range(len(self.tickers)), desc=" Loading price data", leave=False + ) for ticker_type, data in self.tickers.items(): price_data = yf.download( @@ -803,7 +805,7 @@ def __calculate_portfolio_returns(self): from any sales during the period [Cash Inflow(t)]. """ - p_bar = tqdm(range(1), desc=" Calculating returns") + p_bar = tqdm(range(1), desc=" Calculating returns", leave=False) trade_data = self.historical_trade_data From 2ccc503ddf2ea7f21d32b0ce75883f6a908c0634 Mon Sep 17 00:00:00 2001 From: Diogo Sousa Date: Sun, 5 Feb 2023 12:08:13 +0000 Subject: [PATCH 3/7] black --- openbb_terminal/portfolio/portfolio_controller.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openbb_terminal/portfolio/portfolio_controller.py b/openbb_terminal/portfolio/portfolio_controller.py index 14a140d8e7b9..bfd5be83dcbd 100644 --- a/openbb_terminal/portfolio/portfolio_controller.py +++ b/openbb_terminal/portfolio/portfolio_controller.py @@ -440,7 +440,9 @@ def call_bench(self, other_args: List[str]): "[red]Please first load transactions file using 'load'[/red]" ) else: - if self.portfolio.set_benchmark(ns_parser.benchmark, ns_parser.full_shares): + if self.portfolio.set_benchmark( + ns_parser.benchmark, ns_parser.full_shares + ): self.benchmark_name = statics.BENCHMARK_CHOICES.get( ns_parser.benchmark, ns_parser.benchmark ) From db693be042ad1066aeb8a216c4027f9ead71063f Mon Sep 17 00:00:00 2001 From: Diogo Sousa Date: Sun, 5 Feb 2023 13:11:47 +0000 Subject: [PATCH 4/7] handle some error cases --- .../portfolio/test_portfolio.openbb | 2 + openbb_terminal/portfolio/allocation_model.py | 73 +++++++++++-------- .../portfolio/portfolio_controller.py | 30 +++++--- openbb_terminal/portfolio/portfolio_engine.py | 5 +- openbb_terminal/portfolio/portfolio_view.py | 20 ++++- 5 files changed, 83 insertions(+), 47 deletions(-) diff --git a/openbb_terminal/miscellaneous/integration_tests_scripts/portfolio/test_portfolio.openbb b/openbb_terminal/miscellaneous/integration_tests_scripts/portfolio/test_portfolio.openbb index 231ff5618666..572a3955c5bd 100644 --- a/openbb_terminal/miscellaneous/integration_tests_scripts/portfolio/test_portfolio.openbb +++ b/openbb_terminal/miscellaneous/integration_tests_scripts/portfolio/test_portfolio.openbb @@ -2,6 +2,8 @@ portfolio load --example show bench QQQ +bench FAKE_BENCH +bench TSLA alloc assets -t -l 4 alloc sectors -t -l 4 alloc countries -t -l 4 diff --git a/openbb_terminal/portfolio/allocation_model.py b/openbb_terminal/portfolio/allocation_model.py index 9cdf7b933cf1..606f06fdef30 100644 --- a/openbb_terminal/portfolio/allocation_model.py +++ b/openbb_terminal/portfolio/allocation_model.py @@ -72,11 +72,16 @@ def get_assets_allocation( pd.DataFrame DataFrame with the portfolio's asset allocations """ - benchmark_assets_allocation = pd.DataFrame(benchmark_info["holdings"]) - benchmark_assets_allocation.rename( - columns={"symbol": "Symbol", "holdingPercent": "Benchmark"}, inplace=True - ) - benchmark_assets_allocation.drop(columns=["holdingName"], inplace=True) + if "holdings" in benchmark_info: + benchmark_assets_allocation = pd.DataFrame(benchmark_info["holdings"]) + benchmark_assets_allocation.rename( + columns={"symbol": "Symbol", "holdingPercent": "Benchmark"}, inplace=True + ) + benchmark_assets_allocation.drop(columns=["holdingName"], inplace=True) + else: + benchmark_assets_allocation = pd.DataFrame( + columns=["Symbol", "Benchmark", "Sector", "Country"] + ) portfolio_assets_allocation = ( portfolio_trades[portfolio_trades["Type"] != "CASH"] @@ -109,34 +114,37 @@ def get_sectors_allocation( Returns ------- pd.DataFrame - DataFrame with regional allocations. + DataFrame with benchmark allocations. pd.DataFrame - DataFrame with country allocations + DataFrame with portfolio allocations """ - benchmark_sectors_allocation = ( - pd.DataFrame.from_dict( - data={ - sector_name: allocation - for sector in benchmark_info["sectorWeightings"] - for sector_name, allocation in sector.items() - }, - orient="index", + if "sectorWeightings" in benchmark_info: + benchmark_sectors_allocation = ( + pd.DataFrame.from_dict( + data={ + sector_name: allocation + for sector in benchmark_info["sectorWeightings"] + for sector_name, allocation in sector.items() + }, + orient="index", + ) + .squeeze() + .sort_values(ascending=False) ) - .squeeze() - .sort_values(ascending=False) - ) - # Prettify sector allocations of benchmark to align with Portfolio Excel - prettified = [ - sector.replace("_", " ").title() - for sector in benchmark_sectors_allocation.index - ] + # Prettify sector allocations of benchmark to align with Portfolio Excel + prettified = [ + sector.replace("_", " ").title() + for sector in benchmark_sectors_allocation.index + ] - benchmark_sectors_allocation.index = prettified - benchmark_sectors_allocation = pd.DataFrame(benchmark_sectors_allocation) - benchmark_sectors_allocation.reset_index(inplace=True) - benchmark_sectors_allocation.columns = ["Sector", "Benchmark"] + benchmark_sectors_allocation.index = prettified + benchmark_sectors_allocation = pd.DataFrame(benchmark_sectors_allocation) + benchmark_sectors_allocation.reset_index(inplace=True) + benchmark_sectors_allocation.columns = ["Sector", "Benchmark"] + else: + benchmark_sectors_allocation = pd.DataFrame(columns=["Sector", "Benchmark"]) # Define portfolio sector allocation # Aggregate sector value for stocks and crypto @@ -243,9 +251,9 @@ def get_countries_allocation( Returns ------- pd.DataFrame - DataFrame with regional allocations. + DataFrame with benchmark allocations. pd.DataFrame - DataFrame with country allocations + DataFrame with portfolio allocations """ benchmark_allocation = get_symbol_allocation( @@ -305,7 +313,7 @@ def get_symbol_allocation( Returns ------- pd.DataFrame - Dictionary with country allocations + Dictionary with category allocations """ if category == "Region": @@ -332,7 +340,10 @@ def get_symbol_allocation( # Collect data from Fidelity about the portfolio composition of the benchmark URL = f"https://screener.fidelity.com/ftgw/etf/goto/snapshot/portfolioComposition.jhtml?symbols={symbol}" html = request(URL).content - df_list = pd.read_html(html) + try: + df_list = pd.read_html(html) + except ValueError: + return pd.DataFrame(columns=[category, col_name]) # Find the ones that contain regions and countries for index, item in enumerate(df_list): diff --git a/openbb_terminal/portfolio/portfolio_controller.py b/openbb_terminal/portfolio/portfolio_controller.py index bfd5be83dcbd..3551fe12b570 100644 --- a/openbb_terminal/portfolio/portfolio_controller.py +++ b/openbb_terminal/portfolio/portfolio_controller.py @@ -141,6 +141,7 @@ def __init__(self, queue: List[str] = None): self.portfolio_name: str = "" self.benchmark_name: str = "" self.original_benchmark_ticker = "" + self.recalculate_alloc = False self.risk_free_rate = 0 self.portlist: List[str] = os.listdir(self.DEFAULT_HOLDINGS_PATH) self.portfolio = None @@ -447,6 +448,7 @@ def call_bench(self, other_args: List[str]): ns_parser.benchmark, ns_parser.benchmark ) self.original_benchmark_ticker = ns_parser.benchmark + self.recalculate_alloc = True @log_start_end(log=logger) def call_alloc(self, other_args: List[str]): @@ -488,27 +490,31 @@ def call_alloc(self, other_args: List[str]): ): if ns_parser.agg == "assets": portfolio_view.display_assets_allocation( - self.portfolio, - ns_parser.limit, - ns_parser.tables, + portfolio_engine=self.portfolio, + limit=ns_parser.limit, + tables=ns_parser.tables, + recalculate=self.recalculate_alloc, ) elif ns_parser.agg == "sectors": portfolio_view.display_sectors_allocation( - self.portfolio, - ns_parser.limit, - ns_parser.tables, + portfolio_engine=self.portfolio, + limit=ns_parser.limit, + tables=ns_parser.tables, + recalculate=self.recalculate_alloc, ) elif ns_parser.agg == "countries": portfolio_view.display_countries_allocation( - self.portfolio, - ns_parser.limit, - ns_parser.tables, + portfolio_engine=self.portfolio, + limit=ns_parser.limit, + tables=ns_parser.tables, + recalculate=self.recalculate_alloc, ) elif ns_parser.agg == "regions": portfolio_view.display_regions_allocation( - self.portfolio, - ns_parser.limit, - ns_parser.tables, + portfolio_engine=self.portfolio, + limit=ns_parser.limit, + tables=ns_parser.tables, + recalculate=self.recalculate_alloc, ) else: console.print( diff --git a/openbb_terminal/portfolio/portfolio_engine.py b/openbb_terminal/portfolio/portfolio_engine.py index b52f639a5cce..56d3b26410ec 100644 --- a/openbb_terminal/portfolio/portfolio_engine.py +++ b/openbb_terminal/portfolio/portfolio_engine.py @@ -118,7 +118,7 @@ def __init__(self, transactions: pd.DataFrame = pd.DataFrame()): self.tickers_list = None self.tickers: Dict[Any, Any] = {} self.benchmark_ticker: str = "" - self.benchmark_info = None + self.benchmark_info: Dict[Any, Any] = {} self.historical_trade_data = pd.DataFrame() # Portfolio @@ -560,7 +560,8 @@ def set_benchmark(self, symbol: str = "SPY", full_shares: bool = False) -> bool: p_bar.refresh() try: - self.benchmark_info = yf.Ticker(symbol).info + self.benchmark_info = dict(yf.Ticker(symbol).info) + self.benchmark_info["symbol"] = symbol except Exception as _: # noqa console.print( f"[red]\n\nCould not get info for {symbol}." diff --git a/openbb_terminal/portfolio/portfolio_view.py b/openbb_terminal/portfolio/portfolio_view.py index e5734881216d..132ca92a484f 100644 --- a/openbb_terminal/portfolio/portfolio_view.py +++ b/openbb_terminal/portfolio/portfolio_view.py @@ -145,6 +145,7 @@ def display_assets_allocation( portfolio_engine: PortfolioEngine, limit: int = 10, tables: bool = False, + recalculate: bool = False, ): """Display portfolio asset allocation compared to the benchmark @@ -152,16 +153,19 @@ def display_assets_allocation( ---------- portfolio_engine: PortfolioEngine Instance of PortfolioEngine class - tables: bool - Whether to include separate asset allocation tables limit: int The amount of assets you wish to show, by default this is set to 10 + tables: bool + Whether to include separate asset allocation tables + recalculate: bool + Whether to recalculate the allocation """ combined, portfolio_allocation, benchmark_allocation = get_assets_allocation( portfolio_engine=portfolio_engine, limit=limit, tables=True, + recalculate=recalculate, ) display_category( @@ -179,6 +183,7 @@ def display_sectors_allocation( portfolio_engine: PortfolioEngine, limit: int = 10, tables: bool = False, + recalculate: bool = False, ): """Display portfolio sector allocation compared to the benchmark @@ -190,12 +195,15 @@ def display_sectors_allocation( The amount of assets you wish to show, by default this is set to 10 tables: bool Whether to include separate asset allocation tables + recalculate: bool + Whether to recalculate the allocation """ combined, portfolio_allocation, benchmark_allocation = get_sectors_allocation( portfolio_engine=portfolio_engine, limit=limit, tables=True, + recalculate=recalculate, ) display_category( @@ -213,6 +221,7 @@ def display_countries_allocation( portfolio_engine: PortfolioEngine, limit: int = 10, tables: bool = False, + recalculate: bool = False, ): """Display portfolio country allocation compared to the benchmark @@ -224,12 +233,15 @@ def display_countries_allocation( The amount of assets you wish to show, by default this is set to 10 tables: bool Whether to include separate asset allocation tables + recalculate: bool + Whether to recalculate the allocation """ combined, portfolio_allocation, benchmark_allocation = get_countries_allocation( portfolio_engine=portfolio_engine, limit=limit, tables=True, + recalculate=recalculate, ) display_category( @@ -247,6 +259,7 @@ def display_regions_allocation( portfolio_engine: PortfolioEngine, limit: int = 10, tables: bool = False, + recalculate: bool = False, ): """Display portfolio region allocation compared to the benchmark @@ -258,12 +271,15 @@ def display_regions_allocation( The amount of assets you wish to show, by default this is set to 10 tables: bool Whether to include separate asset allocation tables + recalculate: bool + Whether to recalculate the allocation """ combined, portfolio_allocation, benchmark_allocation = get_regions_allocation( portfolio_engine=portfolio_engine, limit=limit, tables=True, + recalculate=recalculate, ) display_category( From 3f9fe8b685f3890b9971512ea3493843e622a08d Mon Sep 17 00:00:00 2001 From: Diogo Sousa Date: Sun, 5 Feb 2023 13:22:02 +0000 Subject: [PATCH 5/7] lower before compare --- openbb_terminal/core/completer/choices.py | 6 +++--- openbb_terminal/decorators.py | 2 +- openbb_terminal/portfolio/portfolio_engine.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openbb_terminal/core/completer/choices.py b/openbb_terminal/core/completer/choices.py index ba9ffbed8341..6d3a58dc01a8 100644 --- a/openbb_terminal/core/completer/choices.py +++ b/openbb_terminal/core/completer/choices.py @@ -214,7 +214,7 @@ def __patch_controller_functions(controller): ), ] - if environ.get("DEBUG_MODE", "false") != "true": + if str(environ.get("DEBUG_MODE", "false")).lower() != "true": rich.start() patched_function_list = [] for patcher in patcher_list: @@ -222,7 +222,7 @@ def __patch_controller_functions(controller): yield patched_function_list - if environ.get("DEBUG_MODE", "false") != "true": + if str(environ.get("DEBUG_MODE", "false")).lower() != "true": rich.stop() for patcher in patcher_list: patcher.stop() @@ -319,7 +319,7 @@ def build_controller_choice_map(controller) -> dict: argument_parser=argument_parser ) except Exception as exception: - if environ.get("DEBUG_MODE", "false") == "true": + if str(environ.get("DEBUG_MODE", "false")).lower() == "true": raise Exception( f"On command : `{command}`.\n{str(exception)}" ) from exception diff --git a/openbb_terminal/decorators.py b/openbb_terminal/decorators.py index f94cbb0227bb..9ced3603f060 100644 --- a/openbb_terminal/decorators.py +++ b/openbb_terminal/decorators.py @@ -60,7 +60,7 @@ def wrapper(*args, **kwargs): extra={"func_name_override": func.__name__}, ) - if os.environ.get("DEBUG_MODE") == "true": + if str(os.environ.get("DEBUG_MODE")).lower() == "true": value = func(*args, **kwargs) log.info("END", extra={"func_name_override": func.__name__}) return value diff --git a/openbb_terminal/portfolio/portfolio_engine.py b/openbb_terminal/portfolio/portfolio_engine.py index 8a4357cec062..22d33b552a2b 100644 --- a/openbb_terminal/portfolio/portfolio_engine.py +++ b/openbb_terminal/portfolio/portfolio_engine.py @@ -195,7 +195,7 @@ def read_transactions(path: str) -> pd.DataFrame: # Load transactions from file if path.endswith(".xlsx"): - if environ.get("DEBUG_MODE", "false") != "true": + if str(environ.get("DEBUG_MODE", "false")).lower() != "true": warnings.filterwarnings( "ignore", category=UserWarning, module="openpyxl" ) From 53bab3962d995a5fe3114921fb46f4b5dd685114 Mon Sep 17 00:00:00 2001 From: jeroen Date: Mon, 6 Feb 2023 13:28:51 +0100 Subject: [PATCH 6/7] Add sheetnames and improve mret --- .../portfolio/portfolio_controller.py | 107 ++++++++++++++---- openbb_terminal/portfolio/portfolio_model.py | 31 ++++- openbb_terminal/portfolio/portfolio_view.py | 76 ++++++++++--- 3 files changed, 177 insertions(+), 37 deletions(-) diff --git a/openbb_terminal/portfolio/portfolio_controller.py b/openbb_terminal/portfolio/portfolio_controller.py index 3551fe12b570..ae29a5e7df2a 100644 --- a/openbb_terminal/portfolio/portfolio_controller.py +++ b/openbb_terminal/portfolio/portfolio_controller.py @@ -968,6 +968,23 @@ def call_mret(self, other_args: List[str]): help="Period to select start end of the year returns", metavar="PERIOD", ) + parser.add_argument( + "-i", + "--instrument", + type=str, + dest="instrument", + default="both", + choices=["both", "portfolio", "benchmark"], + help="Whether to show portfolio or benchmark monthly returns. By default both are shown in one table.", + ) + parser.add_argument( + "-g", + "--graph", + action="store_true", + default=False, + dest="graph", + help="Plot the monthly returns on a heatmap", + ) parser.add_argument( "-s", "--show", @@ -982,7 +999,7 @@ def call_mret(self, other_args: List[str]): parser, other_args, raw=True, - export_allowed=EXPORT_ONLY_FIGURES_ALLOWED, + export_allowed=EXPORT_ONLY_RAW_DATA_ALLOWED, ) if ns_parser and self.portfolio is not None: @@ -992,10 +1009,13 @@ def call_mret(self, other_args: List[str]): portfolio_view.display_monthly_returns( self.portfolio, ns_parser.period, - ns_parser.raw, + ns_parser.instrument, + ns_parser.graph, ns_parser.show_vals, ns_parser.export, - ns_parser.sheet_name, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) @log_start_end(log=logger) @@ -1037,7 +1057,9 @@ def call_dret(self, other_args: List[str]): ns_parser.raw, ns_parser.limit, ns_parser.export, - ns_parser.sheet_name, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) @log_start_end(log=logger) @@ -1059,9 +1081,6 @@ def call_maxdd(self, other_args: List[str]): portfolio_view.display_maximum_drawdown( self.portfolio, export=ns_parser.export, - sheet_name=" ".join(ns_parser.sheet_name) - if ns_parser.sheet_name - else None, ) @log_start_end(log=logger) @@ -1295,18 +1314,26 @@ def call_metric(self, other_args: List[str]): self.portfolio, ns_parser.risk_free_rate / 100, ns_parser.export, - ns_parser.sheet_name, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) elif ns_parser.metric == "sortino": portfolio_view.display_sortino_ratio( self.portfolio, ns_parser.risk_free_rate / 100, ns_parser.export, - ns_parser.sheet_name, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) elif ns_parser.metric == "maxdrawdown": portfolio_view.display_maximum_drawdown_ratio( - self.portfolio, ns_parser.export, ns_parser.sheet_name + self.portfolio, + ns_parser.export, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) elif ns_parser.metric == "rsquare": portfolio_view.display_rsquare( @@ -1318,15 +1345,27 @@ def call_metric(self, other_args: List[str]): ) elif ns_parser.metric == "gaintopain": portfolio_view.display_gaintopain_ratio( - self.portfolio, ns_parser.export, ns_parser.sheet_name + self.portfolio, + ns_parser.export, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) elif ns_parser.metric == "trackerr": portfolio_view.display_tracking_error( - self.portfolio, ns_parser.export, ns_parser.sheet_name + self.portfolio, + ns_parser.export, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) elif ns_parser.metric == "information": portfolio_view.display_information_ratio( - self.portfolio, ns_parser.export, ns_parser.sheet_name + self.portfolio, + ns_parser.export, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) elif ns_parser.metric == "tail": portfolio_view.display_tail_ratio( @@ -1338,30 +1377,52 @@ def call_metric(self, other_args: List[str]): ) elif ns_parser.metric == "commonsense": portfolio_view.display_common_sense_ratio( - self.portfolio, ns_parser.export, ns_parser.sheet_name + self.portfolio, + ns_parser.export, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) elif ns_parser.metric == "jensens": portfolio_view.display_jensens_alpha( self.portfolio, ns_parser.risk_free_rate / 100, ns_parser.export, - ns_parser.sheet_name, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) elif ns_parser.metric == "calmar": portfolio_view.display_calmar_ratio( - self.portfolio, ns_parser.export, ns_parser.sheet_name + self.portfolio, + ns_parser.export, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) elif ns_parser.metric == "kelly": portfolio_view.display_kelly_criterion( - self.portfolio, ns_parser.export, ns_parser.sheet_name + self.portfolio, + ns_parser.export, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) elif ns_parser.metric == "payoff" and self.portfolio is not None: portfolio_view.display_payoff_ratio( - self.portfolio, ns_parser.export, ns_parser.sheet_name + self.portfolio, + ns_parser.export, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) elif ns_parser.metric == "profitfactor" and self.portfolio is not None: portfolio_view.display_profit_factor( - self.portfolio, ns_parser.export, ns_parser.sheet_name + self.portfolio, + ns_parser.export, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) @log_start_end(log=logger) @@ -1401,7 +1462,9 @@ def call_distr(self, other_args: List[str]): ns_parser.period, ns_parser.raw, ns_parser.export, - ns_parser.sheet_name, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) @log_start_end(log=logger) @@ -1448,7 +1511,9 @@ def call_summary(self, other_args: List[str]): ns_parser.period, ns_parser.risk_free_rate / 100, ns_parser.export, - ns_parser.sheet_name, + sheet_name=" ".join(ns_parser.sheet_name) + if ns_parser.sheet_name + else None, ) diff --git a/openbb_terminal/portfolio/portfolio_model.py b/openbb_terminal/portfolio/portfolio_model.py index b5292d41888f..030cf692095d 100644 --- a/openbb_terminal/portfolio/portfolio_model.py +++ b/openbb_terminal/portfolio/portfolio_model.py @@ -371,7 +371,36 @@ def get_monthly_returns( ], ) - return monthly_returns, bench_monthly_returns + years = [ + (year, instrument) + for year in monthly_returns.index + for instrument in ["Portfolio", "Benchmark", "Alpha"] + ] + total_monthly_returns = pd.DataFrame( + np.nan, + columns=monthly_returns.columns, + index=pd.MultiIndex.from_tuples(years, names=["Year", "Instrument"]), + ) + + for year in monthly_returns.index: + for instrument in ["Portfolio", "Benchmark", "Alpha"]: + if instrument == "Portfolio": + total_monthly_returns.loc[ + (year, instrument), monthly_returns.columns + ] = monthly_returns.loc[year].values + elif instrument == "Benchmark": + total_monthly_returns.loc[ + (year, instrument), monthly_returns.columns + ] = bench_monthly_returns.loc[year].values + elif instrument == "Alpha": + total_monthly_returns.loc[ + (year, instrument), monthly_returns.columns + ] = ( + monthly_returns.loc[year].values + - bench_monthly_returns.loc[year].values + ) + + return monthly_returns, bench_monthly_returns, total_monthly_returns @log_start_end(log=logger) diff --git a/openbb_terminal/portfolio/portfolio_view.py b/openbb_terminal/portfolio/portfolio_view.py index 132ca92a484f..ad93590837c2 100644 --- a/openbb_terminal/portfolio/portfolio_view.py +++ b/openbb_terminal/portfolio/portfolio_view.py @@ -475,7 +475,8 @@ def display_yearly_returns( def display_monthly_returns( portfolio_engine: PortfolioEngine, window: str = "all", - raw: bool = False, + instrument: str = "both", + graph: bool = False, show_vals: bool = False, export: str = "", sheet_name: str = None, @@ -490,27 +491,38 @@ def display_monthly_returns( Use `portfolio.load` to create a PortfolioEngine. window : str interval to compare cumulative returns and benchmark - raw : False - Display raw data from cumulative return + instrument : str + Display raw data from cumulative return, default is showing both the portfolio and benchmark in one table show_vals : False Show values on heatmap export : str Export certain type of data + sheet_name : str + Whether to export to a specific sheet name in an Excel file external_axes: plt.Axes Optional axes to display plot on """ - portfolio_returns, benchmark_returns = get_monthly_returns(portfolio_engine, window) + portfolio_returns, benchmark_returns, total_monthly_returns = get_monthly_returns( + portfolio_engine, window + ) - if raw: + if instrument == "portfolio": print_rich_table( portfolio_returns, title="Monthly returns - portfolio [%]", headers=portfolio_returns.columns, show_index=True, ) - console.print("\n") + export_data( + export, + os.path.dirname(os.path.abspath(__file__)), + "mret_portfolio", + portfolio_returns, + sheet_name, + ) + elif instrument == "benchmark": print_rich_table( benchmark_returns, title="Monthly returns - benchmark [%]", @@ -518,7 +530,49 @@ def display_monthly_returns( show_index=True, ) - else: + export_data( + export, + os.path.dirname(os.path.abspath(__file__)), + "mret_benchmark", + benchmark_returns, + sheet_name, + ) + elif instrument == "both": + # Converts multi-index into readable rich table while keeping the multi-index intact for export + total_monthly_returns_rich = total_monthly_returns.copy() + total_monthly_returns_rich.insert( + 0, "Instruments", total_monthly_returns_rich.index.get_level_values(1) + ) + total_monthly_returns_rich.insert( + 0, + "Year", + sum( + [ + [year, "", ""] + for year in total_monthly_returns_rich.index.get_level_values( + 0 + ).unique() + ], + [], + ), + ) + + print_rich_table( + total_monthly_returns_rich, + title="Total Monthly returns [%]", + headers=total_monthly_returns_rich.columns, + show_index=False, + ) + + export_data( + export, + os.path.dirname(os.path.abspath(__file__)), + "mret_total", + total_monthly_returns, + sheet_name, + ) + + if graph or show_vals: if external_axes is None: _, ax = plt.subplots( 2, @@ -564,14 +618,6 @@ def display_monthly_returns( if external_axes is None: theme.visualize_output() - export_data( - export, - os.path.dirname(os.path.abspath(__file__)), - "mret", - portfolio_returns, - sheet_name, - ) - @log_start_end(log=logger) def display_daily_returns( From 7dea88b43eb4fb84bbae739c3d699b1d93cbac56 Mon Sep 17 00:00:00 2001 From: Diogo Sousa Date: Mon, 6 Feb 2023 13:51:36 +0000 Subject: [PATCH 7/7] add test case --- .../portfolio/test_portfolio.openbb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/openbb_terminal/miscellaneous/integration_tests_scripts/portfolio/test_portfolio.openbb b/openbb_terminal/miscellaneous/integration_tests_scripts/portfolio/test_portfolio.openbb index 572a3955c5bd..46f17097e517 100644 --- a/openbb_terminal/miscellaneous/integration_tests_scripts/portfolio/test_portfolio.openbb +++ b/openbb_terminal/miscellaneous/integration_tests_scripts/portfolio/test_portfolio.openbb @@ -8,6 +8,11 @@ alloc assets -t -l 4 alloc sectors -t -l 4 alloc countries -t -l 4 alloc regions -t -l 4 +bench SPY +alloc assets -t -l 4 +alloc sectors -t -l 4 +alloc countries -t -l 4 +alloc regions -t -l 4 attrib attrib --raw attrib --type absolute