From 072937eff5080360d3f8140f92a90677d6550506 2020-05-05 18:31:08 From: Brett Smith Date: 2020-05-05 18:31:08 Subject: [PATCH] books.Loader: New loading strategy. The old loading strategy didn't load options, which yielded some spurious errors. It also created awkward duplication of plugin information in the code as well as the books. Implement a new loading strategy that works by reading one of the "main files" under the books/ subdirectory and includes entries for additional FYs beyond that. This is still not ideal in a lot of ways. In particular, Beancount can't cache any results, causing any load to be slower than it theoretically could be. I expect more commits to follow. But some of them might require restructuring the books, and that should happen separately. --- diff --git a/conservancy_beancount/books.py b/conservancy_beancount/books.py index 6f5b6b33130464039f34e1391ef4e245ba24d128..199b922d785ba9148edffa6a3af6d3541460d441 100644 --- a/conservancy_beancount/books.py +++ b/conservancy_beancount/books.py @@ -14,7 +14,9 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +import contextlib import datetime +import os from pathlib import Path @@ -23,6 +25,7 @@ from beancount import loader as bc_loader from typing import ( Any, Iterable, + Iterator, Mapping, NamedTuple, Optional, @@ -33,9 +36,17 @@ from .beancount_types import ( ) PathLike = Union[str, Path] -PluginsSpec = Mapping[str, Optional[str]] Year = Union[int, datetime.date] +@contextlib.contextmanager +def workdir(path: PathLike) -> Iterator[Path]: + old_dir = os.getcwd() + os.chdir(path) + try: + yield Path(old_dir) + finally: + os.chdir(old_dir) + class FiscalYear(NamedTuple): month: int = 3 day: int = 1 @@ -87,14 +98,9 @@ class FiscalYear(NamedTuple): class Loader: """Load Beancount books organized by fiscal year""" - DEFAULT_PLUGINS: PluginsSpec = { - 'conservancy_beancount.plugin': None, - } - def __init__(self, books_root: Path, fiscal_year: FiscalYear, - plugins: Optional[PluginsSpec]=None, ) -> None: """Set up a books loader @@ -102,69 +108,56 @@ class Loader: * books_root: A Path to a Beancount books checkout. * fiscal_year: A FiscalYear object, used to determine what books to load for a given date range. - * plugins: A mapping that specifies what plugins should be loaded - before any books. The keys are plugin names, and the values are the - configuration parameters string to follow. A value of None means the - plugin takes no configuration string. By default, the loader loads - conservancy_beancount.plugin. """ - if plugins is None: - plugins = self.DEFAULT_PLUGINS self.books_root = books_root + self.opening_root = books_root / 'books' self.fiscal_year = fiscal_year - self.plugins = dict(plugins) - def _format_include(self, year: int, subdir: PathLike='') -> str: - file_path = Path(self.books_root, subdir, f'{year}.beancount') - return f'include "{file_path}"' - - def _format_plugin(self, name: str, optstring: Optional[str]=None) -> str: - if optstring is None: - return f'plugin "{name}"' - else: - return f'plugin "{name}" "{optstring}"' + def _iter_fy_books(self, fy_range: Iterable[int]) -> Iterator[Path]: + dir_path = self.opening_root + for year in fy_range: + path = dir_path / f'{year}.beancount' + if path.exists(): + yield path + dir_path = self.books_root def fy_range_string(self, from_fy: Year, to_fy: Optional[Year]=None, - plugins: Optional[PluginsSpec]=None, ) -> str: """Return a string to load books for a range of fiscal years This method generates a range of fiscal years by calling FiscalYear.range() with its first two arguments. It returns a string of - Beancount directives to load all plugins and Beancount files for that - range of fiscal years, suitable for passing to - beancount.loader.load_string(). + Beancount directives to load the books from the first available fiscal + year through the end of the range. - You can specify what plugins to load with the plugins argument. If not - specified, the string loads the plugins specified for this instance. - See the __init__ docstring for details. + Pass the string to Loader.load_string() to actually load data from it. """ - if plugins is None: - plugins = self.plugins - years = iter(self.fiscal_year.range(from_fy, to_fy)) + paths = self._iter_fy_books(self.fiscal_year.range(from_fy, to_fy)) try: - books_start = self._format_include(next(years), 'books') + with next(paths).open() as opening_books: + lines = [opening_books.read()] except StopIteration: return '' - return '\n'.join([ - *(self._format_plugin(name, opts) for name, opts in plugins.items()), - books_start, - *(self._format_include(year) for year in years), - ]) + for path in paths: + lines.append(f'include "../{path.name}"') + return '\n'.join(lines) + + def load_string(self, source: str) -> LoadResult: + """Load a generated string of Beancount directives + + This method takes a string generated by another Loader method, like + fy_range_string, and loads it through Beancount, setting up the + environment as necessary to do that. + """ + with workdir(self.opening_root): + retval: LoadResult = bc_loader.load_string(source) + return retval def load_fy_range(self, from_fy: Year, to_fy: Optional[Year]=None, - plugins: Optional[PluginsSpec]=None, ) -> LoadResult: - """Load books for a range of fiscal years - - This is a convenience wrapper to call - self.fy_range_string(from_fy, to_fy, plugins) - and load the result with beancount.loader.load_string. - """ - return bc_loader.load_string( # type:ignore[no-any-return] - self.fy_range_string(from_fy, to_fy, plugins), - ) + """Load books for a range of fiscal years""" + return self.load_string(self.fy_range_string(from_fy, to_fy)) diff --git a/tests/books/2018.beancount b/tests/books/2018.beancount new file mode 100644 index 0000000000000000000000000000000000000000..677a9de0dff0769a49da64ddd94067072c711e08 --- /dev/null +++ b/tests/books/2018.beancount @@ -0,0 +1,3 @@ +2018-04-01 * "2018 donation" + Income:Donations 20.18 USD + Assets:Checking diff --git a/tests/books/2019.beancount b/tests/books/2019.beancount new file mode 100644 index 0000000000000000000000000000000000000000..4efd4953c088f314b6d136da17bd1f86b90d185f --- /dev/null +++ b/tests/books/2019.beancount @@ -0,0 +1,4 @@ +2019-04-01 * "2019 donation" + Income:Donations 20.19 USD + Assets:Checking + diff --git a/tests/books/2020.beancount b/tests/books/2020.beancount new file mode 100644 index 0000000000000000000000000000000000000000..f75265fb4d24bfaf53ff2df1017686e8cba1de6c --- /dev/null +++ b/tests/books/2020.beancount @@ -0,0 +1,3 @@ +2020-04-01 * "2020 donation" + Income:Donations 20.20 USD + Assets:Checking diff --git a/tests/books/books/2018.beancount b/tests/books/books/2018.beancount new file mode 100644 index 0000000000000000000000000000000000000000..ca7c2c4b935c1d93ec81a386c2b0696518d63113 --- /dev/null +++ b/tests/books/books/2018.beancount @@ -0,0 +1,3 @@ +option "title" "Books from 2018" +plugin "beancount.plugins.auto" +include "../2018.beancount" diff --git a/tests/books/books/2019.beancount b/tests/books/books/2019.beancount new file mode 100644 index 0000000000000000000000000000000000000000..964f51a0a99e685f2893616d523bc42bd3e8b931 --- /dev/null +++ b/tests/books/books/2019.beancount @@ -0,0 +1,3 @@ +option "title" "Books from 2019" +plugin "beancount.plugins.auto" +include "../2019.beancount" diff --git a/tests/books/books/2020.beancount b/tests/books/books/2020.beancount new file mode 100644 index 0000000000000000000000000000000000000000..79d1fc364b2b5a64b392b33ce18a1c5b539c5065 --- /dev/null +++ b/tests/books/books/2020.beancount @@ -0,0 +1,3 @@ +option "title" "Books from 2020" +plugin "beancount.plugins.auto" +include "../2020.beancount" diff --git a/tests/test_books_loader.py b/tests/test_books_loader.py index 058ad75be59e53cd52ca598759fe769c0b48ea31..02cb5ee94ecedc99481e2848a2a4d6a45e7fc7c0 100644 --- a/tests/test_books_loader.py +++ b/tests/test_books_loader.py @@ -15,6 +15,7 @@ # along with this program. If not, see . import hashlib +import re from datetime import date from pathlib import Path @@ -25,59 +26,54 @@ from . import testutil from conservancy_beancount import books -books_path = testutil.test_path('booksroot') +books_path = testutil.test_path('books') @pytest.fixture(scope='module') def conservancy_loader(): return books.Loader(books_path, books.FiscalYear(3)) -def format_include(year, subdir=''): - path = Path(books_path, subdir, f'{year}.beancount') - return f'include "{path}"' - -def format_plugin(name, optstring=None): - if optstring is None: - return f'plugin "{name}"' - else: - return f'plugin "{name}" "{optstring}"' - -def expect_string(years, plugins={'conservancy_beancount.plugin': None}): - years = iter(years) - year1_s = format_include(next(years), 'books') - return '\n'.join([ - *(format_plugin(name, opts) for name, opts in plugins.items()), - year1_s, - *(format_include(year) for year in years), - ]) +def include_patterns(years, subdir='..'): + for year in years: + path = Path(subdir, f'{year}.beancount') + yield rf'^include "{re.escape(str(path))}"$' @pytest.mark.parametrize('range_start,range_stop,expect_years', [ (2019, 2020, [2019, 2020]), (-1, 2020, [2019, 2020]), - (date(2019, 1, 1), date(2020, 6, 1), range(2018, 2021)), + (10, 2019, [2019, 2020]), + (-10, 2019, [2018, 2019]), + (date(2019, 1, 1), date(2020, 6, 1), [2018, 2019, 2020]), (-1, date(2020, 2, 1), [2018, 2019]), ]) def test_fy_range_string(conservancy_loader, range_start, range_stop, expect_years): - expected = expect_string(expect_years) - assert conservancy_loader.fy_range_string(range_start, range_stop) == expected + actual = conservancy_loader.fy_range_string(range_start, range_stop) + testutil.check_lines_match(actual.splitlines(), [ + rf'^option "title" "Books from {expect_years[0]}"$', + rf'^plugin "beancount\.plugins\.auto"$', + *include_patterns(expect_years), + ]) @pytest.mark.parametrize('year_offset', range(-3, 1)) -def test_fy_range_string_one_offset(conservancy_loader, year_offset): - this_year = date.today().year - expected = expect_string(range(this_year + year_offset, this_year + 1)) - assert conservancy_loader.fy_range_string(year_offset) == expected - -@pytest.mark.parametrize('plugins', [ - {}, - {'conservancy_beancount.plugin': '-all'}, -]) -def test_fy_range_string_plugins_override(conservancy_loader, plugins): - expected = expect_string([2019, 2020], plugins) - assert conservancy_loader.fy_range_string(2019, 2020, plugins) == expected +def test_fy_range_string_with_offset(conservancy_loader, year_offset): + base_year = 2020 + start_year = max(2018, base_year + year_offset) + expect_years = range(start_year, base_year + 1) + actual = conservancy_loader.fy_range_string(year_offset, base_year) + testutil.check_lines_match(actual.splitlines(), include_patterns(expect_years)) def test_fy_range_string_empty_range(conservancy_loader): assert conservancy_loader.fy_range_string(2020, 2019) == '' +def test_load_fy_range(conservancy_loader): + entries, errors, options_map = conservancy_loader.load_fy_range(2018, 2019) + assert not errors + narrations = {getattr(entry, 'narration', None) for entry in entries} + assert '2018 donation' in narrations + assert '2019 donation' in narrations + assert '2020 donation' not in narrations + def test_load_fy_range_empty(conservancy_loader): entries, errors, options_map = conservancy_loader.load_fy_range(2020, 2019) + assert not errors assert not entries assert options_map.get('input_hash') == hashlib.md5().hexdigest() diff --git a/tests/test_config.py b/tests/test_config.py index e18a20eabac7071e05794bc68a93a9f704c15b77..cf05f3612b604e99f187631a477265c7b20a590e 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -383,12 +383,11 @@ def test_default_fiscal_year_begin(): assert actual.day == 1 def test_books_loader(): - books_path = testutil.test_path('bookstest') + books_path = testutil.test_path('books') config = config_mod.Config() config.load_string(f'[Beancount]\nbooks dir = {books_path}\n') loader = config.books_loader() - expected = 'include "{}"'.format(books_path / 'books/2020.beancount') - assert loader.fy_range_string(0, 2020, {}) == expected + assert loader.fy_range_string(2020, 2020) def test_books_loader_without_books(): assert config_mod.Config().books_loader() is None diff --git a/tests/test_reports_accrual.py b/tests/test_reports_accrual.py index eb49c1ed35c7f54c90834018dff4bb2c92175715..d641a5b2ffb4c9b8b0fa92bb7fb7441794ff63f8 100644 --- a/tests/test_reports_accrual.py +++ b/tests/test_reports_accrual.py @@ -273,9 +273,7 @@ def test_consistency_check_when_inconsistent(meta_key, account): def check_output(output, expect_patterns): output.seek(0) - for pattern in expect_patterns: - assert any(re.search(pattern, line) for line in output), \ - f"{pattern!r} not found in output" + testutil.check_lines_match(iter(output), expect_patterns) @pytest.mark.parametrize('invoice,expected', [ ('rt:505/5050', "Zero balance outstanding since 2020-05-05"), diff --git a/tests/testutil.py b/tests/testutil.py index 4d25277be26cf475e106ea6f6e7ab4c7cd614df4..2d05133f9eff9745622ebb224424b3c560829fb1 100644 --- a/tests/testutil.py +++ b/tests/testutil.py @@ -20,6 +20,7 @@ import re import beancount.core.amount as bc_amount import beancount.core.data as bc_data +import beancount.loader as bc_loader from decimal import Decimal from pathlib import Path @@ -33,6 +34,11 @@ FY_MID_DATE = datetime.date(2020, 9, 1) PAST_DATE = datetime.date(2000, 1, 1) TESTS_DIR = Path(__file__).parent +def check_lines_match(lines, expect_patterns, source='output'): + for pattern in expect_patterns: + assert any(re.search(pattern, line) for line in lines), \ + f"{pattern!r} not found in {source}" + def check_post_meta(txn, *expected_meta, default=None): assert len(txn.postings) == len(expected_meta) for post, expected in zip(txn.postings, expected_meta): @@ -180,6 +186,8 @@ class TestBooksLoader(books.Loader): def fy_range_string(self, from_fy=None, to_fy=None, plugins=None): return f'include "{self.source}"' + load_string = staticmethod(bc_loader.load_string) + class TestConfig: def __init__(self, *,