Numerical fixes for UOB importer
kaiwei opened this issue · 12 comments
- "Cash Withdrawal" entries should have 2 decimal places even for whole numbers but are currently missing. This affects automatic numerical display inference in Fava
- Entries should mirror numerical precision from exported Excel bank source, which are to 2 decimal places. But this seems to be rounded down (now) plus the numerical precision seems to go out to several lengthy number of places (which also somehow crashes fava)
- Importer outputs a warning message to terminal line when ran: "WARNING *** file size (92598) not 512 + multiple of sector size (512)"
- Last minor nit tweak: Possible to output the larger positive numberical figures, e.g., 1,000, as it is, without commas (i.e, 1000)? Commas rendering should be something adjustable in fava, etc., but not part of the raw beancount output?
@kaiwei would you mind please attaching these three: anonymized data, expected output, and a minimal config file? This will help debug, for to help turn the debugged cases into unit tests to prevent regressions. Thanks!
I had emailed them to you (earlier) the raw actual files (given privacy concerns). Hope that's okay! Minimal config below:
See below
I totally understand, anonymizing data is a pain. But if you could post even minimal anonymized data here that I can commit to the repository as tests (eg: a single transaction for each issue here), that would be of great help in ensuring things don't break in the future.
IMHO, this is a good opportunity to build and commit tests. Continuing to build and make fixes without unit tests rarely has good outcomes, and I'm simply trying to ensure we end up with a solid, stable codebase with these importers.
I hope that makes sense? Happy to discuss more if needed.
No problems. I'll get to it for the purpose of the tests, but hopefully the actual files (which were emailed) are useful to you in the meantime.
Appreciate the help as always.
Sounds great, thanks much! If it's okay, I'll wait to receive the data to commit the fix to avoid duplication of effort.
Here you go!
Simple config
apply_hooks(uobbank.Importer({
'main_account' : 'Assets:Banks:UOB:UNIPLUS',
'account_number' : '1234567890',
'currency' : 'SGD',
'rounding_error' : 'Equity:Rounding-Errors:Imports',
}), [PredictPostings()]),
- "Cash Withdrawal" entries should have 2 decimal places even for whole numbers but are currently missing. This affects automatic numerical display inference in Fava
Fixed. Still needs a full solution. The problem is, petl uses xlwt/xlrd to read excel, which use excel datatypes, which we don't want, as we want just the string, which we can then pass on to decimal.Decimal. Not sure exactly how decimals are stored in excel
It should have a single digit after the decimal. Please let me know if this solves your issue?
- Entries should mirror numerical precision from exported Excel bank source, which are to 2 decimal places. But this seems to be rounded down (now) plus the numerical precision seems to go out to several lengthy number of places (which also somehow crashes fava)
Fixed.
- Importer outputs a warning message to terminal line when ran: "WARNING *** file size (92598) not 512 + multiple of sector size (512)"
Fixed.
- Last minor nit tweak: Possible to output the larger positive numberical figures, e.g., 1,000, as it is, without commas (i.e, 1000)? Commas rendering should be something adjustable in fava, etc., but not part of the raw beancount output?
Thousands separator-in-source seem to be an issue of personal preference. It improves readability of the source, and some like it for that, particularly if they tend to look at their source a lot. Others don't or don't care.
I'll leave it the way it is to have it remain consistent with beancount_reds_importers. But changing this at your end is trivial: simply run use autobean-format on your source, or pipe the import through it:
autobean-format foo.bean --thousands-separator=add --recursive --output-mode inplace
Thanks for the autobean recommendation! It's a great formatter!
All issues seems resolved for now but instead of a single digit after the decimal, can we pad it to two instead, so that it ends as 1.00 SGD or 1.30 SGD, etc. instead of 1.0 SGD and 1.3 SGD, given dollars and cents are up to two decimals and this also mirrors the root excel file.
Thanks again for the help!
That's the part above I referred to as arising from petl using xlwt/xlrd to read excel, and not yet fully resolved. It might require a bit of digging and time to figure out. I'll do it when I get some time next, but it might be a while.
Aha got it. Thank you for the help!
Piping through a simple sed should fix the issue until resolved.