From 3d704e2865fe30c52f6125ef7752909c0fc9f865 Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Wed, 3 Jun 2020 18:54:12 -0400 Subject: [PATCH] reports: Balance is initialized with just amounts. This works fine with how we're currently using it, makes transformation methods easier to implement, and avoids potential bugs where a balance is initialized with a bad mapping. --- conservancy_beancount/reports/core.py | 43 +++----- tests/test_reports_balance.py | 146 ++++++++++++------------- tests/test_reports_related_postings.py | 5 +- tests/testutil.py | 12 -- 4 files changed, 89 insertions(+), 117 deletions(-) diff --git a/conservancy_beancount/reports/core.py b/conservancy_beancount/reports/core.py index 85cc5ff..c26c43c 100644 --- a/conservancy_beancount/reports/core.py +++ b/conservancy_beancount/reports/core.py @@ -60,27 +60,22 @@ class Balance(Mapping[str, data.Amount]): """ __slots__ = ('_currency_map',) - def __init__(self, - source: Union[Iterable[Tuple[str, data.Amount]], - Mapping[str, data.Amount]]=(), - ) -> None: - if isinstance(source, Mapping): - source = source.items() - self._currency_map = { - currency: amount.number for currency, amount in source - } + def __init__(self, source: Iterable[data.Amount]=()) -> None: + self._currency_map = {amount.currency: amount for amount in source} def _add_amount(self, - currency_map: MutableMapping[str, Decimal], + currency_map: MutableMapping[str, data.Amount], amount: data.Amount, ) -> None: + code = amount.currency try: - currency_map[amount.currency] += amount.number + current_number = currency_map[code].number except KeyError: - currency_map[amount.currency] = amount.number + current_number = Decimal(0) + currency_map[code] = data.Amount(current_number + amount.number, code) def _add_other(self, - currency_map: MutableMapping[str, Decimal], + currency_map: MutableMapping[str, data.Amount], other: Union[data.Amount, 'Balance'], ) -> None: if isinstance(other, Balance): @@ -90,21 +85,19 @@ class Balance(Mapping[str, data.Amount]): self._add_amount(currency_map, other) def __repr__(self) -> str: - return f"{type(self).__name__}({self._currency_map!r})" + values = [repr(amt) for amt in self.values()] + return f"{type(self).__name__}({values!r})" def __str__(self) -> str: return self.format() - def __abs__(self) -> 'Balance': - return type(self)( - (key, bc_amount.abs(amt)) for key, amt in self.items() - ) + def __abs__(self: BalanceType) -> BalanceType: + return type(self)(bc_amount.abs(amt) for amt in self.values()) def __add__(self: BalanceType, other: Union[data.Amount, 'Balance']) -> BalanceType: retval_map = self._currency_map.copy() self._add_other(retval_map, other) - return type(self)((code, data.Amount(number, code)) - for code, number in retval_map.items()) + return type(self)(retval_map.values()) def __eq__(self, other: Any) -> bool: if (self.is_zero() @@ -114,13 +107,11 @@ class Balance(Mapping[str, data.Amount]): else: return super().__eq__(other) - def __neg__(self) -> 'Balance': - return type(self)( - (key, -amt) for key, amt in self.items() - ) + def __neg__(self: BalanceType) -> BalanceType: + return type(self)(-amt for amt in self.values()) def __getitem__(self, key: str) -> data.Amount: - return data.Amount(self._currency_map[key], key) + return self._currency_map[key] def __iter__(self) -> Iterator[str]: return iter(self._currency_map) @@ -132,7 +123,7 @@ class Balance(Mapping[str, data.Amount]): op_func: Callable[[DecimalCompat, DecimalCompat], bool], operand: DecimalCompat, ) -> bool: - return all(op_func(number, operand) for number in self._currency_map.values()) + return all(op_func(amt.number, operand) for amt in self.values()) def eq_zero(self) -> bool: """Returns true if all amounts in the balance == 0.""" diff --git a/tests/test_reports_balance.py b/tests/test_reports_balance.py index d286691..d827018 100644 --- a/tests/test_reports_balance.py +++ b/tests/test_reports_balance.py @@ -34,6 +34,10 @@ DEFAULT_STRINGS = [ ({'JPY': '-5500.00', 'BRL': '-8500.00'}, "-8,500.00 BRL, -5,500 JPY"), ] +def amounts_from_map(currency_map): + for code, number in currency_map.items(): + yield testutil.Amount(number, code) + def test_empty_balance(): balance = core.Balance() assert not balance @@ -49,7 +53,7 @@ def test_empty_balance(): ]) def test_zero_balance(currencies): keys = currencies.split() - balance = core.Balance(testutil.balance_map((key, 0) for key in keys)) + balance = core.Balance(testutil.Amount(0, key) for key in keys) assert balance assert len(balance) == len(keys) assert balance.is_zero() @@ -62,22 +66,22 @@ def test_zero_balance(currencies): 'JPY INR BRL', ]) def test_nonzero_balance(currencies): - amounts = testutil.balance_map(zip(currencies.split(), itertools.count(110, 100))) - balance = core.Balance(amounts.items()) + amounts = dict(zip(currencies.split(), itertools.count(110, 100))) + balance = core.Balance(amounts_from_map(amounts)) assert balance assert len(balance) == len(amounts) assert not balance.is_zero() - assert all(balance[key] == amt for key, amt in amounts.items()) + assert all(balance[key] == testutil.Amount(amt, key) for key, amt in amounts.items()) def test_mixed_balance(): - amounts = testutil.balance_map(USD=0, EUR=120) - balance = core.Balance(amounts.items()) + amounts = {'USD': 0, 'EUR': 120} + balance = core.Balance(amounts_from_map(amounts)) assert balance assert len(balance) == 2 assert not balance.is_zero() - assert all(balance[key] == amt for key, amt in amounts.items()) + assert all(balance[key] == testutil.Amount(amt, key) for key, amt in amounts.items()) -@pytest.mark.parametrize('balance_map_kwargs,expected', [ +@pytest.mark.parametrize('mapping,expected', [ ({}, True), ({'USD': 0}, True), ({'USD': 0, 'EUR': 0}, True), @@ -89,13 +93,12 @@ def test_mixed_balance(): ({'JPY': 10, 'BRL': 0}, False), ({'JPY': 10, 'BRL': 20}, False), ]) -def test_eq_zero(balance_map_kwargs, expected): - amounts = testutil.balance_map(**balance_map_kwargs) - balance = core.Balance(amounts.items()) +def test_eq_zero(mapping, expected): + balance = core.Balance(amounts_from_map(mapping)) assert balance.eq_zero() == expected assert balance.is_zero() == expected -@pytest.mark.parametrize('balance_map_kwargs,expected', [ +@pytest.mark.parametrize('mapping,expected', [ ({}, True), ({'USD': 0}, True), ({'USD': 0, 'EUR': 0}, True), @@ -106,12 +109,11 @@ def test_eq_zero(balance_map_kwargs, expected): ({'JPY': 10, 'BRL': 0}, True), ({'JPY': 10, 'BRL': 20}, True), ]) -def test_ge_zero(balance_map_kwargs, expected): - amounts = testutil.balance_map(**balance_map_kwargs) - balance = core.Balance(amounts.items()) +def test_ge_zero(mapping, expected): + balance = core.Balance(amounts_from_map(mapping)) assert balance.ge_zero() == expected -@pytest.mark.parametrize('balance_map_kwargs,expected', [ +@pytest.mark.parametrize('mapping,expected', [ ({}, True), ({'USD': 0}, True), ({'USD': 0, 'EUR': 0}, True), @@ -122,12 +124,11 @@ def test_ge_zero(balance_map_kwargs, expected): ({'JPY': 10, 'BRL': 0}, False), ({'JPY': 10, 'BRL': 20}, False), ]) -def test_le_zero(balance_map_kwargs, expected): - amounts = testutil.balance_map(**balance_map_kwargs) - balance = core.Balance(amounts.items()) +def test_le_zero(mapping, expected): + balance = core.Balance(amounts_from_map(mapping)) assert balance.le_zero() == expected -@pytest.mark.parametrize('balance_map_kwargs', [ +@pytest.mark.parametrize('mapping', [ {}, {'USD': 0}, {'EUR': 10}, @@ -136,17 +137,13 @@ def test_le_zero(balance_map_kwargs, expected): {'JPY': -25, 'BRL': -35}, {'JPY': 40, 'USD': 0, 'EUR': -50}, ]) -def test_abs(balance_map_kwargs): - amounts = testutil.balance_map(**balance_map_kwargs) - actual = abs(core.Balance(amounts.items())) - assert set(actual) == set(balance_map_kwargs) - abs_amounts = testutil.balance_map(**{ - key: abs(number) for key, number in balance_map_kwargs.items() - }) - for key in balance_map_kwargs: - assert actual[key] == abs_amounts[key] +def test_abs(mapping): + actual = abs(core.Balance(amounts_from_map(mapping))) + assert set(actual) == set(mapping) + for key, number in mapping.items(): + assert actual[key] == testutil.Amount(abs(number), key) -@pytest.mark.parametrize('balance_map_kwargs', [ +@pytest.mark.parametrize('mapping', [ {}, {'USD': 0}, {'EUR': 10}, @@ -155,14 +152,13 @@ def test_abs(balance_map_kwargs): {'JPY': -25, 'BRL': -35}, {'JPY': 40, 'USD': 0, 'EUR': -50}, ]) -def test_neg(balance_map_kwargs): - amounts = testutil.balance_map(**balance_map_kwargs) - actual = -core.Balance(amounts.items()) - assert set(actual) == set(balance_map_kwargs) - for key in balance_map_kwargs: - assert actual[key] == -amounts[key] +def test_neg(mapping): + actual = -core.Balance(amounts_from_map(mapping)) + assert set(actual) == set(mapping) + for key, number in mapping.items(): + assert actual[key] == testutil.Amount(-number, key) -@pytest.mark.parametrize('kwargs1,kwargs2,expected', [ +@pytest.mark.parametrize('map1,map2,expected', [ ({}, {}, True), ({}, {'USD': 0}, True), ({}, {'EUR': 1}, False), @@ -173,9 +169,9 @@ def test_neg(balance_map_kwargs): ({'USD': 1, 'EUR': 2, 'BRL': '3.0'}, {'USD': '1.0', 'EUR': '2.0'}, False), ({'USD': 1, 'EUR': 2}, {'USD': '1.0', 'EUR': '2.0'}, True), ]) -def test_eq(kwargs1, kwargs2, expected): - bal1 = core.Balance(testutil.balance_map(**kwargs1)) - bal2 = core.Balance(testutil.balance_map(**kwargs2)) +def test_eq(map1, map2, expected): + bal1 = core.Balance(amounts_from_map(map1)) + bal2 = core.Balance(amounts_from_map(map2)) actual = bal1 == bal2 assert actual == expected @@ -186,8 +182,8 @@ def test_eq(kwargs1, kwargs2, expected): (-4000, 'BRL'), }) def test_add_amount(number, currency): - start_amounts = testutil.balance_map(USD=500) - start_bal = core.Balance(start_amounts) + start_amount = testutil.Amount(500, 'USD') + start_bal = core.Balance([start_amount]) add_amount = testutil.Amount(number, currency) actual = start_bal + add_amount if currency == 'USD': @@ -195,9 +191,9 @@ def test_add_amount(number, currency): assert actual['USD'] == testutil.Amount(500 + number) else: assert len(actual) == 2 - assert actual['USD'] == testutil.Amount(500) + assert actual['USD'] == start_amount assert actual[currency] == add_amount - assert start_bal == start_amounts + assert start_bal == {'USD': start_amount} @pytest.mark.parametrize('number,currency', { (50, 'USD'), @@ -206,8 +202,7 @@ def test_add_amount(number, currency): (-4000, 'BRL'), }) def test_iadd_amount(number, currency): - start_amounts = testutil.balance_map(USD=500) - balance = core.MutableBalance(start_amounts) + balance = core.MutableBalance([testutil.Amount(500, 'USD')]) add_amount = testutil.Amount(number, currency) balance += add_amount if currency == 'USD': @@ -218,7 +213,7 @@ def test_iadd_amount(number, currency): assert balance['USD'] == testutil.Amount(500) assert balance[currency] == add_amount -@pytest.mark.parametrize('balance_map_kwargs', [ +@pytest.mark.parametrize('mapping', [ {}, {'USD': 0}, {'EUR': 10}, @@ -227,18 +222,17 @@ def test_iadd_amount(number, currency): {'JPY': -25, 'BRL': -35}, {'JPY': 40, 'USD': 0, 'EUR': -50}, ]) -def test_add_balance(balance_map_kwargs): - start_numbers = {'USD': 500, 'BRL': 40000} - start_bal = core.Balance(testutil.balance_map(**start_numbers)) - expect_numbers = start_numbers.copy() - for code, number in balance_map_kwargs.items(): +def test_add_balance(mapping): + expect_numbers = {'USD': 500, 'BRL': 40000} + start_bal = core.Balance(amounts_from_map(expect_numbers)) + for code, number in mapping.items(): expect_numbers[code] = expect_numbers.get(code, 0) + number - add_bal = core.Balance(testutil.balance_map(**balance_map_kwargs)) + add_bal = core.Balance(amounts_from_map(mapping)) actual = start_bal + add_bal - expected = core.Balance(testutil.balance_map(**expect_numbers)) + expected = core.Balance(amounts_from_map(expect_numbers)) assert actual == expected -@pytest.mark.parametrize('balance_map_kwargs', [ +@pytest.mark.parametrize('mapping', [ {}, {'USD': 0}, {'EUR': 10}, @@ -247,25 +241,24 @@ def test_add_balance(balance_map_kwargs): {'JPY': -25, 'BRL': -35}, {'JPY': 40, 'USD': 0, 'EUR': -50}, ]) -def test_iadd_balance(balance_map_kwargs): - start_numbers = {'USD': 500, 'BRL': 40000} - balance = core.MutableBalance(testutil.balance_map(**start_numbers)) - expect_numbers = start_numbers.copy() - for code, number in balance_map_kwargs.items(): +def test_iadd_balance(mapping): + expect_numbers = {'USD': 500, 'BRL': 40000} + balance = core.MutableBalance(amounts_from_map(expect_numbers)) + for code, number in mapping.items(): expect_numbers[code] = expect_numbers.get(code, 0) + number - balance += core.Balance(testutil.balance_map(**balance_map_kwargs)) - expected = core.Balance(testutil.balance_map(**expect_numbers)) + balance += core.Balance(amounts_from_map(mapping)) + expected = core.Balance(amounts_from_map(expect_numbers)) assert balance == expected -@pytest.mark.parametrize('balance_map_kwargs,expected', DEFAULT_STRINGS) -def test_str(balance_map_kwargs, expected): - amounts = testutil.balance_map(**balance_map_kwargs) - assert str(core.Balance(amounts.items())) == expected +@pytest.mark.parametrize('mapping,expected', DEFAULT_STRINGS) +def test_str(mapping, expected): + balance = core.Balance(amounts_from_map(mapping)) + assert str(balance) == expected -@pytest.mark.parametrize('bal_kwargs,expected', DEFAULT_STRINGS) -def test_format_defaults(bal_kwargs, expected): - amounts = testutil.balance_map(**bal_kwargs) - assert core.Balance(amounts).format() == expected +@pytest.mark.parametrize('mapping,expected', DEFAULT_STRINGS) +def test_format_defaults(mapping, expected): + balance = core.Balance(amounts_from_map(mapping)) + assert balance.format() == expected @pytest.mark.parametrize('fmt,expected', [ ('¤##0.0', '¥5000, -€1500.00'), @@ -274,7 +267,7 @@ def test_format_defaults(bal_kwargs, expected): ('#,#00.0 ¤¤;(#,#00.0 ¤¤)', '5,000 JPY, (1,500.00 EUR)'), ]) def test_format_fmt(fmt, expected): - amounts = testutil.balance_map(JPY=5000, EUR=-1500) + amounts = [testutil.Amount(5000, 'JPY'), testutil.Amount(-1500, 'EUR')] balance = core.Balance(amounts) assert balance.format(fmt) == expected @@ -284,16 +277,15 @@ def test_format_fmt(fmt, expected): '\0', ]) def test_format_sep(sep): - bal_kwargs, expected = DEFAULT_STRINGS[-1] + mapping, expected = DEFAULT_STRINGS[-1] expected = expected.replace(', ', sep) - amounts = testutil.balance_map(**bal_kwargs) - balance = core.Balance(amounts) + balance = core.Balance(amounts_from_map(mapping)) assert balance.format(sep=sep) == expected def test_format_none(): - amounts = testutil.balance_map(BRL=65000) - balance = core.Balance(amounts) - expected = babel.numbers.format_currency(65000, 'BRL') + args = (65000, 'BRL') + balance = core.Balance([testutil.Amount(*args)]) + expected = babel.numbers.format_currency(*args) assert balance.format(None) == expected @pytest.mark.parametrize('empty', [ diff --git a/tests/test_reports_related_postings.py b/tests/test_reports_related_postings.py index 6707a24..d014a94 100644 --- a/tests/test_reports_related_postings.py +++ b/tests/test_reports_related_postings.py @@ -90,7 +90,7 @@ def test_balance_credit_card(credit_card_cycle, index, expected): related = core.RelatedPostings( txn.postings[0] for txn in credit_card_cycle[:index + 1] ) - assert related.balance() == testutil.balance_map(USD=expected) + assert related.balance() == {'USD': testutil.Amount(expected, 'USD')} def check_iter_with_balance(entries): expect_posts = [txn.postings[0] for txn in entries] @@ -99,7 +99,8 @@ def check_iter_with_balance(entries): for post in expect_posts: number, currency = post.units balance_tally[currency] += number - expect_balances.append(testutil.balance_map(balance_tally.items())) + expect_balances.append({code: testutil.Amount(number, code) + for code, number in balance_tally.items()}) related = core.RelatedPostings(expect_posts) for (post, balance), exp_post, exp_balance in zip( related.iter_with_balance(), diff --git a/tests/testutil.py b/tests/testutil.py index 3ef2604..e4ee7fc 100644 --- a/tests/testutil.py +++ b/tests/testutil.py @@ -156,18 +156,6 @@ OPENING_EQUITY_ACCOUNTS = itertools.cycle([ 'Equity:OpeningBalance', ]) -def balance_map(source=None, **kwargs): - # The source and/or kwargs should map currency name strings to - # things you can pass to Decimal (a decimal string, an int, etc.) - # This returns a dict that maps currency name strings to Amount instances. - retval = {} - if source is not None: - retval.update((currency, Amount(number, currency)) - for currency, number in source) - if kwargs: - retval.update(balance_map(kwargs.items())) - return retval - def OpeningBalance(acct=None, **txn_meta): if acct is None: acct = next(OPENING_EQUITY_ACCOUNTS)