diff --git a/conservancy_beancount/reconcile/statement_reconciler.py b/conservancy_beancount/reconcile/statement_reconciler.py index 6d3c964..5ba86f8 100644 --- a/conservancy_beancount/reconcile/statement_reconciler.py +++ b/conservancy_beancount/reconcile/statement_reconciler.py @@ -104,12 +104,15 @@ import logging import os import re import sys -from typing import Callable, Dict, List, Tuple, TextIO +from typing import Callable, Dict, List, Optional, Sequence, Tuple, TextIO from beancount import loader from beancount.query.query import run_query from colorama import Fore, Style # type: ignore +from .. import cliutil +from .. import config as configmod + if not sys.warnoptions: import warnings # Disable annoying warning from thefuzz prompting for a C extension. The @@ -117,12 +120,12 @@ if not sys.warnoptions: warnings.filterwarnings('ignore', category=UserWarning, module='thefuzz.fuzz') from thefuzz import fuzz # type: ignore -logger = logging.getLogger() -logger.setLevel(logging.INFO) - -# Console logging. -logger.addHandler(logging.StreamHandler()) +PROGNAME = 'reconcile-statement' +logger = logging.getLogger(__name__) +# Get some interesting feedback on call to RT with this: +# logger.setLevel(logging.DEBUG) +# logger.addHandler(logging.StreamHandler()) JUNK_WORDS = [ 'software', @@ -457,8 +460,16 @@ def parse_repo_relative_path(path: str) -> str: return path -def parse_args(argv: List[str]) -> argparse.Namespace: - parser = argparse.ArgumentParser(description='Reconciliation helper') +def parse_decimal_with_separator(number_text: str) -> decimal.Decimal: + """decimal.Decimal can't parse numbers with thousands separator.""" + number_text = number_text.replace(',', '') + return decimal.Decimal(number_text) + + +def parse_arguments(argv: List[str]) -> argparse.Namespace: + parser = argparse.ArgumentParser(prog=PROGNAME, description='Reconciliation helper') + cliutil.add_version_argument(parser) + cliutil.add_loglevel_argument(parser) parser.add_argument('--beancount-file', required=True, type=parse_path) parser.add_argument('--csv-statement', required=True, type=parse_repo_relative_path) parser.add_argument('--bank-statement', required=True, type=parse_repo_relative_path) @@ -466,9 +477,10 @@ def parse_args(argv: List[str]) -> argparse.Namespace: parser.add_argument('--grep-output-filename') # parser.add_argument('--report-group-regex') parser.add_argument('--show-reconciled-matches', action='store_true') - parser.add_argument('--statement-balance', type=decimal.Decimal, required=True, help="A.K.A \"cleared balance\" taken from the end of the period on the PDF statement. Required because CSV statements don't include final or running totals") + parser.add_argument('--statement-balance', type=parse_decimal_with_separator, required=True, help="A.K.A \"cleared balance\" taken from the end of the period on the PDF statement. Required because CSV statements don't include final or running totals") parser.add_argument('--non-interactive', action='store_true', help="Don't prompt to write to the books") - return parser.parse_args(args=argv[1:]) + args = parser.parse_args(args=argv) + return args def totals(matches: List[Tuple[List, List, List]]) -> Tuple[decimal.Decimal, decimal.Decimal, decimal.Decimal]: @@ -512,9 +524,6 @@ def subset_match(statement_trans: List[dict], books_trans: List[dict]) -> Tuple[ matches.append(([statement_trans[best_match_index]], group_items, best_match_note)) if best_match_index is not None: del statement_trans[best_match_index] - for item in group_items: - # TODO: Why? - books_trans.remove(item) else: remaining_books_trans.append(r2) for r1 in statement_trans: @@ -531,7 +540,17 @@ def process_unmatched(statement_trans: List[dict], books_trans: List[dict]) -> L return matches -def main(args: argparse.Namespace) -> None: +def main(arglist: Optional[Sequence[str]] = None, + stdout: TextIO = sys.stdout, + stderr: TextIO = sys.stderr, + config: Optional[configmod.Config] = None, + ) -> int: + args = parse_arguments(arglist) + cliutil.set_loglevel(logger, args.loglevel) + if config is None: + config = configmod.Config() + config.load_file() + # TODO: Should put in a sanity check to make sure the statement you're feeding # in matches the account you've provided. @@ -607,10 +626,7 @@ def main(args: argparse.Namespace) -> None: write_metadata_to_books(metadata_to_apply) -if __name__ == '__main__': - args = parse_args(sys.argv) - main(args) +entry_point = cliutil.make_entry_point(__name__, PROGNAME) -def entry_point(): - args = parse_args(sys.argv) - main(args) +if __name__ == '__main__': + exit(entry_point()) diff --git a/tests/test_reconcile.py b/tests/test_reconcile.py index b7085a3..48835b1 100644 --- a/tests/test_reconcile.py +++ b/tests/test_reconcile.py @@ -308,3 +308,21 @@ def test_subset_sum_match(): [], # No remaining statement trans. [], # No remaining books trans. ) + +def test_subset_passes_through_all_non_matches(): + """This was used to locate a bug where some of the non-matches had + gone missing due to mutation of books_trans.""" + statement_trans = [ + S1, # No match + S4, # Match + ] + books_trans = [ + B2, # No match + B4A, B4B, B4C, # Match + B3_next_day, B3_next_week, # No match + ] + assert subset_match(statement_trans, books_trans) == ( + [([S4], [B4A, B4B, B4C], [])], # Matched + [S1], # No match: preserved intact + [B2, B3_next_day, B3_next_week] # No match: preserved intact + )