Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

matching.py: Fix incorrect fuzzy amount loop bounds #210

Merged
merged 2 commits into from
Nov 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion beancount_import/matching.py
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,11 @@ def _get_matches(
if cur_matches is not None:
lower_bound = bisect.bisect_left(cur_matches, (lower, tuple(), None, None))
upper_bound = bisect.bisect_right(cur_matches, (upper, (sys.maxsize,), None, None), lo=lower_bound)
for sp in cur_matches[lower_bound-1 if lower_bound > 0 else 0:upper_bound]:
for sp in cur_matches[lower_bound:upper_bound]:
assert abs(sp.number - amount.number) <= self.fuzzy_match_amount, (
f'Bug in matching algorithm: {sp} is not within '
+ f'{self.fuzzy_match_amount} of {amount}')

posting = sp.mp.posting
# Verify that the account is compatible.
if not are_accounts_mergeable(account, posting.account):
Expand Down
70 changes: 70 additions & 0 deletions beancount_import/matching_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -721,8 +721,47 @@ def test_match_fuzzy_amount():
note: "B"
""")

def test_match_fuzzy_amount_upper_bound():
# We match despite a skew of 0.01 USD, our configured fuzzy_match_amount.
assert_match(
pending_candidate="""
2016-01-01 * "Narration"
note: "A"
Income:A -100 USD
note: "A"
Expenses:FIXME 100 USD
""",
pending="""
2016-01-01 * "Narration"
note2: "B"
Assets:B 100.01 STOCK { 1.00 USD }
note: "B"
Expenses:FIXME -100.01 USD
""",
matches="""
2016-01-01 * "Narration"
note: "A"
note2: "B"
Income:A -100 USD
note: "A"
Assets:B 100.01 STOCK { 1.00 USD }
note: "B"
""")

def test_nonmatch_fuzzy_amount():
# We don't match with a skew of 0.02, beyond our configured fuzzy_match_amount.
#
# This test case searches for matches for the "Expenses:FIXME 100 USD"
# posting among the following 4 candidates, ordered by amount:
#
# 1. Income:A -100 USD
# 2. Expenses:FIXME -99.98 USD
# 3. Assets:B 99.98 STOCK { 1.00 USD }
# 4. Expenses:FIXME 100 USD
#
# Only posting #1 should be considered; the others are beyond the
# fuzzy_match_amount. But since posting #1 is from the same transaction as the
# original posting, it should be excluded and no match should be returned.
assert_match(
pending_candidate="""
2016-01-01 * "Narration"
Expand All @@ -739,6 +778,37 @@ def test_nonmatch_fuzzy_amount():
Expenses:FIXME -99.98 USD
""")

def test_nonmatch_fuzzy_amount_2():
# We don't match with a skew of 0.02, beyond our configured fuzzy_match_amount.
#
# This test case searches for matches for the "Expenses:FIXME -20.00 USD"
# posting among the following 4 candidates, ordered by amount:
#
# 1. Expenses:FIXME -20.00 USD
# 2. Assets:B -19.98 USD
# 3. Expenses:FIXME 19.98 USD
# 4. Assets:A 20.00 USD
#
# Only posting #4 should be considered; the others are beyond the
# fuzzy_match_amount. But since posting #4 is from the same transaction as the
# original posting, it should be excluded and no match should be returned.
#
# The difference between this test case and `test_nonmatch_fuzzy_amount()`
# above is the position of the target posting within the candidates list: at
# the end of the list rather than the beginning. This test case reproduces
# issue #202.
assert_match(
pending_candidate="""
2023-01-01 * "Transaction 0"
Assets:A 20.00 USD
Expenses:FIXME -20.00 USD
""",
pending="""
2023-01-01 * "Transaction 1"
Assets:B -19.98 USD
Expenses:FIXME 19.98 USD
""")

def test_match_grouped_differing_signs():
# Can group postings of differing signs to make a match.
assert_match(
Expand Down
Loading