search

simonmichael / hledger

Robust, fast, intuitive plain text accounting tool with CLI, TUI and web interfaces.
https://hledger.org
GNU General Public License v3.0
2.98k stars 318 forks source link

transaction modifiers miss inferred amounts in transaction #893

Open jkr opened 6 years ago

jkr commented 6 years ago

Running with a stack build of up-to-date master.

Transaction modifiers seem to interact oddly with inferred amounts. The following works as expected:

= ^expenses:groceries
    [budget:groceries]                           *-1
    [assets:bank:checking]                        *1

10/7 * MARKET
    expenses:groceries:food                      $20
    assets:bank:checking
$ hledger -ffoo.ledger reg --auto
2018/10/07 MARKET               ex:groceries:food              $20           $20
                                [budget:groceries]            $-20             0
                                [as:bank:checking]             $20           $20
                                assets:bank:checking          $-20             0

But the following does not:

= ^expenses:groceries
    [budget:groceries]                           *-1
    [assets:bank:checking]                        *1

10/7 * MARKET
    expenses:groceries:food
    assets:bank:checking                         $-20
$ hledger -ffoo.ledger reg --auto
hledger: could not balance this transaction - can't have more than one balanced virtual posting with no amount (remember to put 2 or more spaces before amounts)
2018/10/07 * MARKET
    expenses:groceries:food
    [budget:groceries]
    [assets:bank:checking]
    assets:bank:checking                $-20
simonmichael commented 6 years ago

Thanks for the report.

jkr commented 6 years ago

I'll poke around as a way of familiarizing myself with the codebase, and see if I can help out. Would HLedger.Data.TransactionModifer be where I should start? (It looks like the inference logic is in H.D.Transaction, right?)

simonmichael commented 6 years ago

@jkr, that would be great. Yes that sounds right.

jkr commented 5 years ago

I figured out the issue: it was applying the modifiers before it finalized the journal with journalFinalise. A simple solution (apply modifiers after journalFinalise) is here:

[EDIT: this solution doesn't work, for the reasons discussed in this post. There is a better solution in the post below.]

https://github.com/simonmichael/hledger/compare/master...jkr:modifiers_fix

Now this works:

= ^expenses:groceries
    [budget:groceries]                           *-1
    [assets:bank:checking]                        *1

10/7 * MARKET
    expenses:groceries:food                      $20
    assets:bank:checking

10/8 * MARKET2
    expenses:groceries:food
    assets:bank:checking                        -$20
$ stack exec hledger -- print -f /tmp/foo.ledger --auto
2018/10/07 * MARKET
    expenses:groceries:food             $20
    [budget:groceries]                 $-20
    [assets:bank:checking]              $40
    assets:bank:checking               $-20

$ stack exec hledger -- print -f /tmp/foo.ledger --auto
2018/10/07 * MARKET
    expenses:groceries:food             $20
    [budget:groceries]                 $-20
    [assets:bank:checking]              $20
    assets:bank:checking

2018/10/08 * MARKET2
    expenses:groceries:food
    [budget:groceries]                 $-20
    [assets:bank:checking]              $20
    assets:bank:checking               $-20

BUT, this produces its own problem. Because the application happens after the finalization, it no longer checks to make sure they balance. So we can now have:

= ^expenses:groceries
    [budget:groceries]                           *-1
    [assets:bank:checking]                        *2

10/7 * MARKET
    expenses:groceries:food                      $20
    assets:bank:checking                        -$20
$ stack exec hledger -- print -f /tmp/foo.ledger --auto
2018/10/07 * MARKET
    expenses:groceries:food             $20
    [budget:groceries]                 $-20
    [assets:bank:checking]              $40
    assets:bank:checking               $-20

So it seems like the best option would be to run finalize twice: before and after modification. This makes the most intuitive sense since the journal should work both with and without --auto, so it's essentially checking two different journals. I'm not sure how expensive this would be, but the second run would only occur if auto_ iopts, so if you don't run specifiy auto you won't see it.

I could also try to split out infernce from finalize, but I imagine there are other checks that would end up missing, so running it twice if you have auto seems like the most direct route.

Thoughts?

Also: when do you use doctests and when do you use easytests? I'll add tests when i make a PR but I'm not sure what you usually do.

jkr commented 5 years ago

Okay, this implements the double-run I discussed above:

https://github.com/simonmichael/hledger/compare/master...jkr:modifiers_fix_2

What do you think?

simonmichael commented 5 years ago

That sounds reasonable.

jkr commented 5 years ago

OK -- I put up a PR. There's no testing in at the moment, since I couldn't figure out the proper place to put this test in the current framework. If you have a suggestion, I'll add it to the PR.

jkr commented 5 years ago

I see from the Travis output where the relevant tests are. I'll try to address those and add to the PR.

simonmichael commented 5 years ago

Seems good to me! Thanks for the fix.

thargor commented 5 years ago

Hi @jkr , after talking to @simonmichael on irc we decided to document this here and maybe reopen this issue:

My use case is double checking tax calculations by supplying the net amount, gross amount and taxation rules as tags.

The following example works without these pull requests (<1.12) and breaks after (>= 1.12).

example.txt

= tag:tax20
  taxes  *0.2

2018/12/18 transaction with taxes
  a    EUR -10.00    ; :tax20:
  b    EUR 12.00

before 1.12

$ ./hledger.exe -f  example.txt bal --auto
          EUR -10.00  a
           EUR 12.00  b
           EUR -2.00  taxes
--------------------
                   0

after 1.12

hledger.exe: "example.txt" (lines 5-7)
could not balance this transaction (real postings are off by EUR 2.00)
2018/12/18 transaction with taxes
    a      EUR -10.00    ; :tax20:
    b       EUR 12.00

Before 1.12 it was not required to have the transactions balance before adding the automated transactions and this worked.

thargor commented 5 years ago

This does also not work with 1.12:

= tag:tax20  
  taxes  *0.2

2018/12/18 transaction with taxes
  a    EUR -10.00    ; :tax20:
  b
simonmichael commented 5 years ago

Thanks for these examples @thargor. Here they are in another form:

A. Both amounts explicit:

= tag:tax20
  taxes  *0.2

2018/12/18 transaction with taxes
  a    EUR -10.00    ; :tax20:
  b    EUR 12.00

# 1. hledger 1.11 accepts this, with --auto
$ hledger-1.11 print -x --auto
2018/12/18 transaction with taxes
    a          EUR -10.00    ; :tax20:
    taxes       EUR -2.00
    b           EUR 12.00

# 2. hledger 1.12 doesn't
$ hledger-1.12 print -x --auto
hledger-1.12: "/Users/simon/src/PLAINTEXTACCOUNTING/hledger/b.j" (lines 5-7)
could not balance this transaction (real postings are off by EUR 2.00)
2018/12/18 transaction with taxes
    a      EUR -10.00    ; :tax20:
    b       EUR 12.00

B. One amount implicit:

= tag:tax20  
  taxes  *0.2

2018/12/18 transaction with taxes
  a    EUR -10.00    ; :tax20:
  b

# 3. hledger 1.11 accepts this with or without --auto
$ hledger-1.11 print -x
2018/12/18 transaction with taxes
    a      EUR -10.00    ; :tax20:
    b       EUR 10.00

# 4.
$ hledger-1.11 print -x --auto
2018/12/18 transaction with taxes
    a          EUR -10.00    ; :tax20:
    taxes       EUR -2.00
    b           EUR 12.00

# 5. hledger 1.12 accepts it only without --auto
$ hledger-1.12 print -x
2018/12/18 transaction with taxes
    a      EUR -10.00    ; :tax20:
    b       EUR 10.00

# 6.
$ hledger-1.12 print -x --auto
hledger-1.12: "/Users/simon/src/PLAINTEXTACCOUNTING/hledger/c.j" (lines 5-7)
could not balance this transaction (real postings are off by EUR -2.00)
2018/12/18 transaction with taxes
    a          EUR -10.00    ; :tax20:
    taxes       EUR -2.00
    b           EUR 10.00

@thargor always runs with --auto, and wanted to write both amounts explicitly for clarity/error checking (example 1).

@jkr's original examples above seemed sensible, but now the failing examples 2 and 6 with hledger 1.12 do seem a bit wrong. Perhaps we need to rethink this change. Even revert it ? To do something in time for tomorrow's release, it will have to happen soon. @jkr or anyone, any thoughts ?

simonmichael commented 5 years ago

the journal should work both with and without --auto

Is this possible/desirable as a general rule ? It seems not possible for some journals, eg A above (unless you did a looser check, trying to balance transactions both with and without --auto and accepting either.)

simonmichael commented 5 years ago

1.11 required transactions to balance after any auto postings had been added. 1.12+ requires transactions to also balance before auto postings are added.

simonmichael commented 5 years ago

To comply with that, here's how @thargor would have to write that journal A:

C. balanced both before and after auto postings added

= tag:tax20
  taxes   *0.2
  b      *-0.2

2018/12/18 transaction with taxes
  a    EUR -10.00    ; :tax20:
  b     EUR 10.00

# 7.
$ hledger-1.12 print -x
2018/12/18 transaction with taxes
    a      EUR -10.00    ; :tax20:
    b       EUR 10.00

# 8.
$ hledger-1.12 print -x --auto
2018/12/18 transaction with taxes
    a          EUR -10.00    ; :tax20:
    taxes       EUR -2.00
    b            EUR 2.00
    b           EUR 10.00
thargor commented 5 years ago 
= tag:tax20
  taxes   *0.2
  b      *-0.2

I cannot use this, since b (and also a) is not a fixed accout name, but depends on the transaction.

2018/12/18 transaction with taxes 1
  a    EUR -10.00    ; :tax20:
  b     EUR 10.00

2018/12/18 transaction with taxes 2
  c    EUR -10.00    ; :tax20:
  d     EUR 10.00
simonmichael commented 5 years ago

I cannot use this, since b (and also a) is not a fixed accout name, but depends on the transaction.

Could be handled with multiple more specific rules, I assume:

= tag:tax20  desc:'taxes 1'
  taxes   *0.2
  b      *-0.2

= tag:tax20  c  ; or whatever
  taxes   *0.3
  d      *-0.3

Unrelated: I see you're using Ledger tag syntax - hledger doesn't require that leading colon.

simonmichael commented 5 years ago

Current behaviour noted in docs: https://hledger.org/hledger.html#auto-postings-and-transaction-balancing--inferred-amounts--balance-assertions

Xitian9 commented 3 years ago

This issue is still open. Is there still something that needs to be fixed/clarified here?

simonmichael commented 3 years ago

We documented and have heard nothing more in 2.5 years, hopefully the current behaviour is the best solution.

simonmichael commented 2 years ago

Hi all. Recently two Ledger users asked about this (auto postings + balance assignments) restriction in hledger. Would you have any thoughts to add to https://www.reddit.com/r/plaintextaccounting/comments/tpaak1/hledgers_issues_with_my_virtual_transactions or my comment ? Especially @jkr who looked most closely at this (I know some time has passed so it might have faded :-). Are some things worth reconsidering here ?