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

Predictions might lead to original posting being switched to 2nd place after prediction #128

Open
awtimmering opened this issue Apr 5, 2024 · 0 comments · May be fixed by #129
Open

Predictions might lead to original posting being switched to 2nd place after prediction #128

awtimmering opened this issue Apr 5, 2024 · 0 comments · May be fixed by #129

Comments

@awtimmering
Copy link

awtimmering commented Apr 5, 2024

I've had a few instances where predicted postings lead to an unnatural posting order (and I have at least 2 people confirming having seen the same). For example, the output of:

2017-01-07 * "Groceries"
  Assets:US:BofA:Checking  -10.20 USD

in certain circumstances can lead to a predicted posting of

2017-01-07 * "Groceries"
  Expenses:Groceries
  Assets:US:BofA:Checking  -10.20 USD

with the filler in the first slot, and the original posting account being pushed to 2nd slot.

Having stepped through the code as this happens, it does seem to be just a prediction fluke that can occur, where accounts can end up in this order as result of predictor.py#L234.

This seems it can be easily addressed with a somewhat simplified update_postings(transaction, accounts) (entries.py#L6) to honor the original order, i.e. always start with posting[0] and augment with any predicted accounts, but am not 100% confident I understand the intent of the original logic in update_postings() and whether this change would upset existing work flows. PR attached with unit tests.

awtimmering added a commit to awtimmering/smart_importer that referenced this issue Apr 5, 2024
@awtimmering awtimmering changed the title Predictions might lead to original posting being siwtched to 2nd place post-prediction Predictions might lead to original posting being switched to 2nd place after prediction Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant