From be87d6b1d1cb0987b913a2ad60aba7ed99660093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Mari=C4=87?= Date: Sat, 10 Apr 2021 02:46:11 -0700 Subject: [PATCH] Throw an error message for invalid tickers + allow decimal values for sum to invest (#6) * Throw an error message for invalid tickers - Print an error message when the portfolio CSV-file contains tickers that don't exist on Yahoo finance. - Allow decimal values to be used for the sum to invest input. * Bump up version number Bump up version number in anticipation for version 1.0.1. --- portfolio_rebalancer/cli.py | 2 +- portfolio_rebalancer/portfolio.py | 4 ++ portfolio_rebalancer/portfolio_csv_reader.py | 13 ++++--- pyproject.toml | 2 +- tests/test_portfolio_rebalancer.py | 41 ++++++++++++++++---- 5 files changed, 47 insertions(+), 15 deletions(-) diff --git a/portfolio_rebalancer/cli.py b/portfolio_rebalancer/cli.py index 2e839e0..9dcc5a6 100644 --- a/portfolio_rebalancer/cli.py +++ b/portfolio_rebalancer/cli.py @@ -13,7 +13,7 @@ def portfolio_rebalancer(): "your portfolio as close as possible to " "your target allocation given a sum to " "invest.")) -@click.argument('sum-to-invest', type=click.INT) +@click.argument('sum-to-invest', type=click.FLOAT) @click.option('-p', '--portfolio', 'portfolio_csv', required=True, help="CSV-file containing the portfolio and target allocations.") def calc(sum_to_invest, portfolio_csv): diff --git a/portfolio_rebalancer/portfolio.py b/portfolio_rebalancer/portfolio.py index 4eb5586..f4b13de 100644 --- a/portfolio_rebalancer/portfolio.py +++ b/portfolio_rebalancer/portfolio.py @@ -8,6 +8,10 @@ def __init__(self): def __getitem__(self, asset_name): return self._assets[asset_name] + @property + def assets(self): + return self._assets + def add_asset(self, asset): self._total += asset.price * asset.qty self._assets[asset.name] = asset diff --git a/portfolio_rebalancer/portfolio_csv_reader.py b/portfolio_rebalancer/portfolio_csv_reader.py index 2b0843b..c0bc85f 100644 --- a/portfolio_rebalancer/portfolio_csv_reader.py +++ b/portfolio_rebalancer/portfolio_csv_reader.py @@ -39,7 +39,6 @@ def _sanitize_target_allocation(self, target_allocation, line_num): def get_portfolio(self): portfolio = Portfolio() total_allocation_pct = 0.0 - seen_assets = set() with open(self.portfolio_csv, newline='') as f: reader = csv.reader(f) for line_num, row in enumerate(reader): @@ -52,9 +51,13 @@ def get_portfolio(self): shares = self._sanitize_shares(row[1], line_num) target_allocation = self._sanitize_target_allocation(row[2], line_num) - asset = PortfolioAsset(name, shares, target_allocation) - if name not in seen_assets: - seen_assets.add(name) + try: + asset = PortfolioAsset(name, shares, target_allocation) + except KeyError: + raise ClickException( + "Row {} - ticker {} doesn't exist".format(line_num, + name)) + if name not in portfolio.assets: portfolio.add_asset(asset) else: raise ClickException("Asset name {} appears twice on row " @@ -62,5 +65,5 @@ def get_portfolio(self): total_allocation_pct += target_allocation if total_allocation_pct != 100: raise ClickException("Total combined allocation percentage of all " - "rows is over 100%.") + "rows is over 100%") return portfolio diff --git a/pyproject.toml b/pyproject.toml index f2d9e56..098cb0c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "portfolio-rebalancer" -version = "1.0.0" +version = "1.0.1" description = "A CLI tool that shows you what to buy to rebalance your portfolio as best as possible to your target allocations." license = "MIT" readme = "README.md" diff --git a/tests/test_portfolio_rebalancer.py b/tests/test_portfolio_rebalancer.py index e32f0cc..89f5844 100644 --- a/tests/test_portfolio_rebalancer.py +++ b/tests/test_portfolio_rebalancer.py @@ -27,7 +27,7 @@ def test_multiple_buys(testfiles_dir, ticker_mock): data=test_portfolio) runner = CliRunner() - result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '100'], + result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '100.00'], catch_exceptions=False) assert result.exit_code == 0 @@ -59,7 +59,7 @@ def test_same_drift(testfiles_dir, ticker_mock): data=test_portfolio) runner = CliRunner() - result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30'], + result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30.00'], catch_exceptions=False) assert result.exit_code == 0 @@ -85,7 +85,7 @@ def test_malformed_shares_value(testfiles_dir, ticker_mock): data=test_portfolio) runner = CliRunner() - result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30']) + result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30.00']) assert result.exit_code == 1 assert result.output == """\ @@ -109,7 +109,7 @@ def test_malformed_target_allocation_value(testfiles_dir, ticker_mock): data=test_portfolio) runner = CliRunner() - result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30']) + result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30.00']) assert result.exit_code == 1 assert result.output == """\ @@ -136,7 +136,7 @@ def test_same_asset_multiple_times(testfiles_dir, ticker_mock): data=test_portfolio) runner = CliRunner() - result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30']) + result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30.00']) assert result.exit_code == 1 assert result.output == """\ @@ -162,11 +162,11 @@ def test_portfolio_target_allocation_over_100(testfiles_dir, ticker_mock): data=test_portfolio) runner = CliRunner() - result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30']) + result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30.00']) assert result.exit_code == 1 assert result.output == """\ -Error: Total combined allocation percentage of all rows is over 100%. +Error: Total combined allocation percentage of all rows is over 100% """ @@ -180,10 +180,35 @@ def test_no_assets(testfiles_dir, ticker_mock): "test_portfolio.csv", data=test_portfolio) runner = CliRunner() - result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30'], + result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30.00'], catch_exceptions=False) assert result.exit_code == 1 assert result.output == """\ Error: Row 0 malformed - expecting row in this format: TICKER SHARES_OWNED TARET_ALLOCATION """ # noqa: E501 + + +def test_keyerror(testfiles_dir, mocker): + """Test that if the portfolio CSV-file contains a ticker that can't be + found on Yahoo finance. + """ + test_portfolio = [ + ['MFST', '1', '100'] + ] + test_portfolio_csv = create_csv_file(testfiles_dir, + "test_portfolio.csv", + data=test_portfolio) + + mock_obj = mocker.patch( + 'portfolio_rebalancer.portfolio_asset.yf.Ticker.info') + mock_obj.__getitem__.side_effect = KeyError + + runner = CliRunner() + result = runner.invoke(pr, ['calc', '-p', test_portfolio_csv, '30.00'], + catch_exceptions=False) + + assert result.exit_code == 1 + assert result.output == """\ +Error: Row 0 - ticker MFST doesn't exist +""" # noqa: E501