Skip to content

"balance" parser for csv reader#537

Merged
simonmichael merged 2 commits into
simonmichael:masterfrom
adept:master
Apr 14, 2017
Merged

"balance" parser for csv reader#537
simonmichael merged 2 commits into
simonmichael:masterfrom
adept:master

Conversation

@adept

@adept adept commented Apr 13, 2017

Copy link
Copy Markdown
Collaborator

Ability to add balance assertions in csv reader

@simonmichael

Copy link
Copy Markdown
Owner

Nice! I ask myself how much benefit this brings, but I currently put paypal's balance in a transaction comment for troubleshooting/informational purposes, so I guess why not check it as well, given that the performance impact seems quite small.

But as you mentioned in IRC, ordering issues might be a problem. We might get multiple same-day transactions, and we can't rely on the ordering of same-day CSV records. We might also see multiple same-account postings in a transaction (if not now, then in future). How easy/efficient would it be to omit the assertions in just these cases ?

@simonmichael

Copy link
Copy Markdown
Owner

PS I'd guess trying to re-order things automatically to make assertions work would bring headaches. We don't want to risk changing the meaning of the data.

@adept

adept commented Apr 14, 2017

Copy link
Copy Markdown
Collaborator Author

I've played a bit more with this patch, and I am liking it more and more.

It is highly unlikely that within singe CSV file that has balance there would be out-of-order transactions. In only encounetered issues with multiple current accounts in a single bank, where each account have its own CSV file and transfers between account are therefore present in both.

Previously i've been dropping one transfer transaction out of two, pretty much at random, and this is what played havoc with balance assertions. Once I kept both of them, writing a rule so that one transfer is getting imported as "account A -> special account for transfers" and second transfer is "special account for transfers -> account B", my balances got in order and every assertion checked out.

I no longer believe that my reordering idea is necessary at all, and you are right - it is unnecessart complicated.

@simonmichael simonmichael added A-WISH Some kind of improvement request or proposal. csv The csv file format, csv output format, or generally CSV-related. labels Apr 14, 2017
@simonmichael

Copy link
Copy Markdown
Owner

Ok, let's merge this and people can try it. In case of trouble they can not use it or do manual fixups. I think it just needs some docs.

@adept

adept commented Apr 14, 2017

Copy link
Copy Markdown
Collaborator Author

I've added a bit of documentation

@simonmichael
simonmichael merged commit 451f9d7 into simonmichael:master Apr 14, 2017
@simonmichael

Copy link
Copy Markdown
Owner

Looks good, thanks!

@simonmichael

Copy link
Copy Markdown
Owner

Some issues noticed recently:

  • in several rules files I already had a balance field, which I ignored or included as a comment. After upgrading hledger from 1.2 to HEAD those conversions all broke, eg because the bank sometimes leaves the balance field blank.

  • balance assertions aren't valid in the converted journal, because the CSV was just a recent subset of transactions. If you want to run hledger against the converted journal, you have to remember to add -I/--ignore-assertions.

@simonmichael

Copy link
Copy Markdown
Owner

PS so the "If you assign empty string to it, no assertion will be generated" case mentioned in the docs, doesn't seem to be working here.

@adept

adept commented Apr 18, 2017 via email

Copy link
Copy Markdown
Collaborator Author

@adept

adept commented Apr 18, 2017

Copy link
Copy Markdown
Collaborator Author

To be precise, I previously tested that this will work:

if
match for transaction that needs to have balance assertion excluded
balance

But I failed to test that empty or all-space value for balance will work. #539 should address this.

@simonmichael

Copy link
Copy Markdown
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-WISH Some kind of improvement request or proposal. csv The csv file format, csv output format, or generally CSV-related.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants