From 1fc9363b26cba6b2e1b29d5f9601be96b3ed5a4e 2020-03-31 14:07:25 From: Brett Smith Date: 2020-03-31 14:07:25 Subject: [PATCH] data: Add is_credit() and is_debit() methods to Posting. The main motivation for this change is to make sure that higher-level code deals with the fact that self.units.number can be None, and has an easy way to do so. I'm not sure all our code is *currently* doing the right thing for this case, because I'm not sure it will ever actually come up. It's possible that earlier Beancount plugins fill in decimal amounts for postings that are originally loaded with self.units.number=None. I'll have to see later whether this case comes up in reality, and then deal with it if so. For now the safest strategy seems to be that most code should operate when self.units.number is None. --- diff --git a/conservancy_beancount/data.py b/conservancy_beancount/data.py index b8bd4ccd1b16e79027b01fcacaadb497c3d7a031..6509ab0438d46e54f4b51da106f7f582beac4da8 100644 --- a/conservancy_beancount/data.py +++ b/conservancy_beancount/data.py @@ -21,11 +21,13 @@ throughout Conservancy tools. import collections import decimal +import operator from beancount.core import account as bc_account from typing import ( cast, + Callable, Iterable, Iterator, MutableMapping, @@ -215,13 +217,33 @@ class Posting(BasePosting): # If it did, this declaration would pass without issue. meta: Metadata # type:ignore[assignment] - def is_payment(self, threshold: DecimalCompat=0) -> bool: - threshold = cast(decimal.Decimal, threshold) - return ( - self.account.is_real_asset() - and self.units.number is not None - and self.units.number < -abs(threshold) - ) + def _compare_amount(self, + op: Callable[[decimal.Decimal], decimal.Decimal], + threshold: DecimalCompat, + default: Optional[bool], + ) -> Optional[bool]: + if self.units.number is None: + return default + else: + return op(self.units.number) > threshold + + def is_credit(self, + threshold: DecimalCompat=0, + default: Optional[bool]=None, + ) -> Optional[bool]: + return self._compare_amount(operator.pos, threshold, default) + + def is_debit(self, + threshold: DecimalCompat=0, + default: Optional[bool]=None, + ) -> Optional[bool]: + return self._compare_amount(operator.neg, threshold, default) + + def is_payment(self, + threshold: DecimalCompat=0, + default: Optional[bool]=None, + ) -> Optional[bool]: + return self.account.is_real_asset() and self.is_debit(threshold, default) def iter_postings(txn: Transaction) -> Iterator[Posting]: diff --git a/conservancy_beancount/plugin/meta_approval.py b/conservancy_beancount/plugin/meta_approval.py index 809ffd009261c6a1b82388e21cfc882feea148e2..15188474092999dcb9b953b0cf3b5d571b6df29b 100644 --- a/conservancy_beancount/plugin/meta_approval.py +++ b/conservancy_beancount/plugin/meta_approval.py @@ -29,7 +29,7 @@ class MetaApproval(core._RequireLinksPostingMetadataHook): CREDIT_CARD_ACCT = 'Liabilities:CreditCard' def __init__(self, config: configmod.Config) -> None: - self.payment_threshold = -abs(config.payment_threshold()) + self.payment_threshold = config.payment_threshold() def _run_on_txn(self, txn: Transaction) -> bool: if not super()._run_on_txn(txn): @@ -37,11 +37,11 @@ class MetaApproval(core._RequireLinksPostingMetadataHook): assets_sum = decimal.Decimal(0) creditcard_sum = decimal.Decimal(0) for post in data.iter_postings(txn): - if post.is_payment(self.payment_threshold): - assets_sum += post.units.number or 0 + if post.is_payment(): + assets_sum -= post.units.number or 0 elif post.account.is_under(self.CREDIT_CARD_ACCT): creditcard_sum += post.units.number or 0 - return (assets_sum + creditcard_sum) < 0 + return (assets_sum - creditcard_sum) > self.payment_threshold def _run_on_post(self, txn: Transaction, post: data.Posting) -> bool: - return post.is_payment() + return post.is_payment(0) is not False diff --git a/conservancy_beancount/plugin/meta_tax_implication.py b/conservancy_beancount/plugin/meta_tax_implication.py index 3a74f45d15390d01532a5c36cb1c7e46aa2a99aa..154fb101f3c7ade705601e99e3f8623a873e00df 100644 --- a/conservancy_beancount/plugin/meta_tax_implication.py +++ b/conservancy_beancount/plugin/meta_tax_implication.py @@ -48,4 +48,4 @@ class MetaTaxImplication(core._NormalizePostingMetadataHook): self.payment_threshold = config.payment_threshold() def _run_on_post(self, txn: Transaction, post: data.Posting) -> bool: - return post.is_payment(self.payment_threshold) + return post.is_payment(self.payment_threshold) is not False diff --git a/tests/test_data_posting.py b/tests/test_data_posting.py index b9ee686cf755e80fed4dd72d0ce161ac316428ac..5f6dc05b18547dc793e86020bf4c889aca37a74f 100644 --- a/tests/test_data_posting.py +++ b/tests/test_data_posting.py @@ -39,25 +39,64 @@ NON_PAYMENT_ACCOUNTS = { 'UnearnedIncome:MatchPledges', } +AMOUNTS = [ + None, + '-25.50', + 0, + '25.75', +] + def Posting(account, number, currency='USD', cost=None, price=None, flag=None, **meta): if not meta: meta = None + if number is not None: + number = Decimal(number) return data.Posting( data.Account(account), - bc_amount.Amount(Decimal(number), currency), + bc_amount.Amount(number, currency), cost, price, flag, meta, ) -def check_all_thresholds(post, threshold, expected): - assert post.is_payment(threshold) is expected - assert post.is_payment(-threshold) is expected - assert post.is_payment(Decimal(threshold)) is expected - assert post.is_payment(Decimal(-threshold)) is expected +def check_all_thresholds(expected, method, threshold, *args): + assert method(threshold, *args) is expected + assert method(Decimal(threshold), *args) is expected + +@pytest.mark.parametrize('amount', AMOUNTS) +def test_is_credit(amount): + expected = None if amount is None else float(amount) > 0 + assert Posting('Assets:Cash', amount).is_credit() is expected + +def test_is_credit_threshold(): + post = Posting('Assets:Cash', 25) + check_all_thresholds(True, post.is_credit, 0) + check_all_thresholds(True, post.is_credit, 20) + check_all_thresholds(False, post.is_credit, 40) + +def test_is_credit_default(): + post = Posting('Assets:Cash', None) + assert post.is_credit(default=True) is True + assert post.is_credit(default=False) is False + +@pytest.mark.parametrize('amount', AMOUNTS) +def test_is_debit(amount): + expected = None if amount is None else float(amount) < 0 + assert Posting('Assets:Cash', amount).is_debit() is expected + +def test_is_debit_threshold(): + post = Posting('Assets:Cash', -25) + check_all_thresholds(True, post.is_debit, 0) + check_all_thresholds(True, post.is_debit, 20) + check_all_thresholds(False, post.is_debit, 40) + +def test_is_debit_default(): + post = Posting('Assets:Cash', None) + assert post.is_debit(default=True) is True + assert post.is_debit(default=False) is False @pytest.mark.parametrize('acct', PAYMENT_ACCOUNTS) def test_is_payment(acct): @@ -71,24 +110,24 @@ def test_is_payment(acct): def test_is_not_payment_account(acct, amount, threshold): post = Posting(acct, -amount) assert not post.is_payment() - check_all_thresholds(post, threshold, False) + check_all_thresholds(False, post.is_payment, threshold) @pytest.mark.parametrize('acct', PAYMENT_ACCOUNTS) def test_is_payment_with_threshold(acct): threshold = len(acct) * 10 post = Posting(acct, -500) - check_all_thresholds(post, threshold, True) + check_all_thresholds(True, post.is_payment, threshold) @pytest.mark.parametrize('acct', PAYMENT_ACCOUNTS) def test_is_not_payment_by_threshold(acct): threshold = len(acct) * 10 post = Posting(acct, -9) - check_all_thresholds(post, threshold, False) + check_all_thresholds(False, post.is_payment, threshold) @pytest.mark.parametrize('acct', PAYMENT_ACCOUNTS) def test_is_not_payment_but_credit(acct): post = Posting(acct, 9) assert not post.is_payment() - check_all_thresholds(post, 0, False) - check_all_thresholds(post, 5, False) - check_all_thresholds(post, 10, False) + check_all_thresholds(False, post.is_payment, 0) + check_all_thresholds(False, post.is_payment, 5) + check_all_thresholds(False, post.is_payment, 10)