-
Notifications
You must be signed in to change notification settings - Fork 38
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
smart_importer with bean-extract patch will post to first posting account despite 'filing_account' #97
Comments
In addition, investment accounts get their file name from the ticker account, and not from the parent account. |
Probably unrelated, would you mind opening a separate bug for this? |
From here. See this comment. This line should be made the default for all transaction builders. |
@redstreet That's the temporary change I tried in my local repo, for banking and investment accounts. Would it be better to change the parent class instead? |
Correct. Patch coming in a few minutes. |
Does that fix work? |
I stepped through the smart_importer code, and it seems to be just a fluke of the prediction (maybe in situations where input data is not very rich yet). I verified the training data does not contain any reversed postings and it outputs it nonethless as result of predictor.py#L234. We could consider proposing a change to |
Good to know nothing in particular caused this. Looks like we (reds-importers) were depending on the ordering so far, that we shouldn't have. In general, ordering of postings in the Beancount ecosystem is not guaranteed AFAIK, so we shouldn't depend on that, or ask others to, as it would be prone to breakage. I also like the idea of printing out a sorted order (even though it makes no functional difference), which we can only do if we have a different way of marking the file to write the transaction to. The above patch I committed yesterday should resolve the problem in a dependable way. Have you tried it? |
Agree. I think regardless of the fix on your side (which is good) it might be a good suggestion for smart_importers, so will suggest & add PR. There might be other considerations, but the current result ends up with postings that are hard to read, even if functionally not different, as the "remainder" entry can come in first place. For example, an import for a bank transaction might end up like this:
Sorry to say I haven't, as for a mix of reasons I am not using your importers (yet, anyway) - even though I have drawn a lot of inspiration from your posts and scripts! (My Japanese bank CSVs don't have headers, then my Japanese Amex CSV does have headers but has multiple sections for family cards that require some custom parsing etc). |
Thank you for the fix, it works well on my side, and it also fixes #98. |
Great! Thanks for reporting. |
+1. I too find a lot of value in friendly posting ordering. After all, a core benefit of plain text accounting is the journal being human readable. Thanks for filing this with smart_importer. |
Glad to hear the articles helped inspire! The process of writing those articles actually helped me get my 5-minute update down to just a few seconds, reliably. So it's well worth it IMHO. Regardless of whether you use my importers: if you don't have headers, Does your Amex contain multiple tables? If so, check out the multitable reader in my importers, either for use or for inspiration. Also based on |
Added issue & PR at beancount/smart_importer#128 . Thank you for the comments - I've been looking at |
Filing_account has no effect for either banking or investment importers.
Using smart_importer together with the patched bean-extract leads to file names related to the first posting in each transaction entry, which is not necessarily the asset account.
The text was updated successfully, but these errors were encountered: