data.Amount: Introduce class and simplify code to use it.

See docstring for full rationale. This greatly reduces the need for other
plugin code to handle the case of `post.units.number is None`, eliminating
the need for entire methods and letting it do plain numeric comparisons.

conservancy_beancount/data.py

@@ -24,6 +24,7 @@ import decimal
 import operator
 
 from beancount.core import account as bc_account
+from beancount.core import amount as bc_amount
 
 from typing import (
     cast,
@@ -114,6 +115,22 @@ class Account(str):
         return None
 
 
+class Amount(bc_amount.Amount):
+    """Beancount amount after processing
+
+    Beancount's native Amount class declares number to be Optional[Decimal],
+    because the number is None when Beancount first parses a posting that does
+    not have an amount, because the user wants it to be automatically balanced.
+
+    As part of the loading process, Beancount replaces those None numbers
+    with the calculated amount, so it will always be a Decimal. This class
+    overrides the type declaration accordingly, so the type checker knows
+    that our code doesn't have to consider the possibility that number is
+    None.
+    """
+    number: decimal.Decimal
+
+
 class Metadata(MutableMapping[MetaKey, MetaValue]):
     """Transaction or posting metadata
 
@@ -221,11 +238,14 @@ class Posting(BasePosting):
     specific fields are replaced with enhanced versions:
 
     * The `account` field is an Account object
+    * The `units` field is our Amount object (which simply declares that the
+      number is always a Decimal—see that docstring for details)
     * The `meta` field is a PostingMeta object
     """
     __slots__ = ()
 
     account: Account
+    units: Amount
     # mypy correctly complains that our MutableMapping is not compatible
     # with Beancount's meta type declaration of Optional[Dict]. IMO
     # Beancount's type declaration is a smidge too specific: I think its type
@@ -234,56 +254,21 @@ class Posting(BasePosting):
     # If it did, this declaration would pass without issue.
     meta: Metadata  # type:ignore[assignment]
 
-    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_cash_equivalent() and self.is_debit(threshold, default)
-
 
 def balance_of(txn: Transaction,
                *preds: Callable[[Account], Optional[bool]],
-               default: Optional[DecimalCompat]=None,
+) -> decimal.Decimal:
-) -> Optional[decimal.Decimal]:
     """Return the balance of specified postings in a transaction.
 
     Given a transaction and a series of account predicates, balance_of
     returns the balance of the amounts of all postings with accounts that
     match any of the predicates.
-
-    If any of the postings have no amount, returns default.
     """
-    retval = decimal.Decimal(0)
+    return sum(
-    for post in txn.postings:
+        (post.units.number for post in iter_postings(txn)
-        acct = Account(post.account)
+         if any(pred(post.account) for pred in preds)),
-        if any(p(acct) for p in preds):
+        decimal.Decimal(0),
-            if post.units.number is None:
+    )
-                return None if default is None else decimal.Decimal(default)
-            else:
-                retval += post.units.number
-    return retval
 
 def iter_postings(txn: Transaction) -> Iterator[Posting]:
     """Yield an enhanced Posting object for every posting in the transaction"""

conservancy_beancount/plugin/meta_approval.py

@@ -31,17 +31,17 @@ class MetaApproval(core._RequireLinksPostingMetadataHook):
         self.payment_threshold = -config.payment_threshold()
 
     def _run_on_txn(self, txn: Transaction) -> bool:
-        if not super()._run_on_txn(txn):
+        return (
-            return False
+            super()._run_on_txn(txn)
             # approval is required when funds leave a cash equivalent asset,
             # UNLESS that transaction is a transfer to another asset,
             # or paying off a credit card.
-        balance = data.balance_of(
+            and self.payment_threshold > data.balance_of(
                 txn,
                 data.Account.is_cash_equivalent,
                 data.Account.is_credit_card,
             )
-        return balance is None or balance < self.payment_threshold
+        )
 
     def _run_on_post(self, txn: Transaction, post: data.Posting) -> bool:
-        return post.account.is_cash_equivalent() and not post.is_credit(0)
+        return post.account.is_cash_equivalent() and post.units.number < 0

conservancy_beancount/plugin/meta_payable_documentation.py

@@ -27,6 +27,6 @@ class MetaPayableDocumentation(core._RequireLinksPostingMetadataHook):
 
     def _run_on_post(self, txn: Transaction, post: data.Posting) -> bool:
         if post.account.is_under('Liabilities:Payable:Accounts'):
-            return not post.is_credit()
+            return post.units.number < 0
         else:
             return False

conservancy_beancount/plugin/meta_receipt.py

@@ -39,7 +39,6 @@ class MetaReceipt(core._RequireLinksPostingMetadataHook):
         return (
             (post.account.is_cash_equivalent() or post.account.is_credit_card())
             and not post.account.is_under('Assets:PayPal')
-            and post.units.number is not None
             and abs(post.units.number) >= self.payment_threshold
         )
 
@@ -66,10 +65,10 @@ class MetaReceipt(core._RequireLinksPostingMetadataHook):
     def post_run(self, txn: Transaction, post: data.Posting) -> errormod.Iter:
         keys = list(self.CHECKED_METADATA)
         is_checking = post.account.is_checking()
-        if is_checking and post.is_debit():
+        if is_checking and post.units.number < 0:
             return self._run_checking_debit(txn, post)
         elif is_checking:
             keys.append('check')
-        elif post.account.is_credit_card() and not post.is_credit():
+        elif post.account.is_credit_card() and post.units.number <= 0:
             keys.append('invoice')
         return self._check_metadata(txn, post, keys)

conservancy_beancount/plugin/meta_receivable_documentation.py

@@ -54,7 +54,7 @@ class MetaReceivableDocumentation(core._RequireLinksPostingMetadataHook):
     def _run_on_post(self, txn: Transaction, post: data.Posting) -> bool:
         if not post.account.is_under('Assets:Receivable'):
             return False
-        elif post.is_debit():
+        elif post.units.number < 0:
             return False
 
         # Get the first invoice, or return False if it doesn't exist.

conservancy_beancount/plugin/meta_tax_implication.py

@@ -45,7 +45,10 @@ class MetaTaxImplication(core._NormalizePostingMetadataHook):
     ])
 
     def __init__(self, config: configmod.Config) -> None:
-        self.payment_threshold = config.payment_threshold()
+        self.payment_threshold = -config.payment_threshold()
 
     def _run_on_post(self, txn: Transaction, post: data.Posting) -> bool:
-        return post.is_payment(self.payment_threshold) is not False
+        return (
+            post.account.is_cash_equivalent()
+            and post.units.number < self.payment_threshold
+        )

tests/test_data_balance_of.py

@@ -34,23 +34,6 @@ def payable_payment_txn():
         ('Assets:Checking', -5),
     ])
 
-@pytest.fixture
-def none_posting_txn():
-    return testutil.Transaction(postings=[
-        ('Income:Donations', -30),
-        ('Expenses:BankingFees', 3),
-        ('Assets:Checking', None),
-    ])
-
-@pytest.fixture
-def multipost_one_none_txn():
-    return testutil.Transaction(postings=[
-        ('Liabilities:Payable:Accounts', 50),
-        ('Assets:Checking', -50),
-        ('Expenses:BankingFees', 5),
-        ('Assets:Checking', None),
-    ])
-
 def balance_under(txn, *accts):
     pred = methodcaller('is_under', *accts)
     return data.balance_of(txn, pred)
@@ -82,25 +65,3 @@ def test_multiarg_balance_of():
 
 def test_balance_of_multipost_txn(payable_payment_txn):
     assert data.balance_of(payable_payment_txn, is_cash_eq) == -55
-
-def test_balance_of_none_posting(none_posting_txn):
-    assert data.balance_of(none_posting_txn, is_cash_eq) is None
-
-def test_balance_of_none_posting_with_default(none_posting_txn):
-    expected = Decimal('Infinity')
-    assert expected == data.balance_of(
-        none_posting_txn, is_cash_eq, default=expected,
-    )
-
-def test_balance_of_other_side_of_none_posting(none_posting_txn):
-    assert balance_under(none_posting_txn, 'Income') == -30
-    assert balance_under(none_posting_txn, 'Expenses') == 3
-
-def test_balance_of_multi_postings_one_none(multipost_one_none_txn):
-    assert data.balance_of(multipost_one_none_txn, is_cash_eq) is None
-
-def test_balance_of_multi_postings_one_none(multipost_one_none_txn):
-    expected = Decimal('Infinity')
-    assert expected == data.balance_of(
-        multipost_one_none_txn, is_cash_eq, default=expected,
-    )

tests/test_data_posting.py

@@ -1,133 +0,0 @@
-"""Test Posting class"""
-# Copyright © 2020  Brett Smith
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU Affero General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU Affero General Public License for more details.
-#
-# You should have received a copy of the GNU Affero General Public License
-# along with this program.  If not, see <https://www.gnu.org/licenses/>.
-
-import pytest
-
-from . import testutil
-
-from decimal import Decimal
-
-import beancount.core.amount as bc_amount
-
-from conservancy_beancount import data
-
-PAYMENT_ACCOUNTS = {
-    'Assets:Cash',
-    'Assets:Bank:Checking',
-}
-
-NON_PAYMENT_ACCOUNTS = {
-    'Assets:Prepaid:Expenses',
-    'Assets:Prepaid:Vacation',
-    'Assets:Receivable:Accounts',
-    'Equity:OpeningBalance',
-    'Expenses:Other',
-    'Income:Other',
-    'Liabilities:CreditCard',
-}
-
-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(number, currency),
-        cost,
-        price,
-        flag,
-        meta,
-    )
-
-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):
-    assert Posting(acct, -500).is_payment()
-
-@pytest.mark.parametrize('acct,amount,threshold', testutil.combine_values(
-    NON_PAYMENT_ACCOUNTS,
-    range(5, 20, 5),
-    range(0, 30, 10),
-))
-def test_is_not_payment_account(acct, amount, threshold):
-    post = Posting(acct, -amount)
-    assert not post.is_payment()
-    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(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(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(False, post.is_payment, 0)
-    check_all_thresholds(False, post.is_payment, 5)
-    check_all_thresholds(False, post.is_payment, 10)