simonmichael / hledger

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

amount %amt @@ -%amt is not allowed when %amt is negative; CSV import #2162

Open PSLLSP opened 9 months ago

PSLLSP commented 9 months ago
$ hledger --version
hledger 1.32.3, linux-x86_64

@@ cannot parse negative amounts (@@ -%amt results in @@ --123.45 and that is a problem for a parser)

Error:

$ hledger -f demo-eur.csv print
hledger: Error: error: could not parse "--2 345.56 USD @@ --2 456.78 EUR" as an amount
CSV record: "2023-01-25","test3","-2 345.56","USD","-2 456.78","EUR"
the account3 rule is: equity:conversion
the account2 rule is: assets:cash
the amount2 rule is: -%3 %4 @@ -%5 %6
the account1 rule is: expenses:assorted
the amount1 rule is: %3 %4 @@ %5 %6
the amount rule is: %5 %6
the comment rule is: \n%1,%2,%3,%4,%5,%6
the date rule is: %1
the description rule is: DEMO|%2

the parse error is:      1:14:
  |
1 | 2 345.56 USD @@ --2 456.78 EUR
  |              ^
unexpected '@'
expecting end of input, ledger-style lot cost, ledger-style lot date, ledger-style lot note, or valuation expression

you may need to change your amount*, balance*, or currency* rules, or add or change your skip rule

CSV file to replicate the bug. The first 2 lines are imported without issue but third line is troublemaker because it has negative numbers:

$ cat demo-eur.csv
2023-01-23,test1,1 234.56,EUR,1 234.56,EUR
2023-01-24,test2,2 345.56,USD,2 456.78,EUR
2023-01-25,test3,-2 345.56,USD,-2 456.78,EUR
2023-01-26,test4,+2 345.56,USD,+2 456.78,EUR

CSV rules import file:

$ cat demo-eur.csv.rules 
# hledger import rules, test

date  %1
description DEMO|%2

amount %5 %6
#amount %6 %5    # to test other issue (-123 -> EUR --123)

amount1 %3 %4 @@ %5 %6
amount2 -%3 %4 @@ -%5 %6
#amount2 %4 -%3 @@ %6 -%5  # similar issue (-123 EUR -> EUR --123)

account1 expenses:assorted
account2 assets:cash
account3 equity:conversion

# debug
comment \n%1,%2,%3,%4,%5,%6

Even positive numbers could be troublemakers:

$ grep "test4" demo-eur.csv
2023-01-26,test4,+2 345.56,USD,+2 456.78,EUR
simonmichael commented 3 months ago

Sorry I missed this report!

I think it's normal that the journal parser rejects these unusual signs in a cost amount. Though perhaps it should be made to accept +, and the parse error message could be improved.

The special handling for --, +, and () is done in the csv reader, which runs first.
And https://hledger.org/hledger.html#amount-signs says

This only works for whole amounts, not for cost amounts such as COST in amount1 AMT @ COST

So it's Functioning As Documented.

I can see it would be consistent to allow this in cost amounts also, but I am guessing it might be a little bit hard to implement; you'd probably have to do it within the (complex) amount parser itself, in the journal reader.

But has this come up in real world data ?

PSLLSP commented 3 months ago

But has this come up in real world data ?

I found this issue when I tried to import real CSV file, it was a file from bank related to a credit card. I do not remember details now, I opened this issue a half year ago. I assume I have found some workaround. I think that my CSV file had records similar to "test3" example and when I tried to find a way to rewrite my rules I have found other issues.


UPDATE, I have not finished my import rules and I see new issue now... ;-)

I define account3 but never amount3 and I think that hledger calculates wrong value for amount3; this is visible in "reg" report.

fields cardid, owner, date1, date2, desc1, amt2, cur2, amt, cur, bal, cur3, desc

amount1  %amt %cur
amount2  -%amt %cur

account1 liabilities:bank:card
account2 expenses:card
account3 equity:conversion

# cur3 is a field in CSV file but amount3 is not defined in my rules

if
%cur2 EUR
%cur2 USD
  amount2 -%amt2 %cur2 @@ %amt %cur

I fill only account1 and account2, account3 (equity:conversion) has "empty" value, so calculation of this value was left on hledger but it doesn't work...

KyleBarton commented 3 months ago

@simonmichael re:

This only works for whole amounts, not for cost amounts such as COST in amount1 AMT @ COST

I'm a little confused by this because the following does negate the absolute cost in this example:

test.csv

2024-01-01, you bought foo, FOO, 10, -100, -1000
2024-01-01, you sold foo, FOO, 10, 100, 1000

test.csv.rules

fields date, description, symbol, quantity, price_per_share, amount

account1 Assets:Test
account2 Assets:Test
amount1 %quantity %symbol @@ -$%amount
amount2 $%amount

$ hledger -f test.csv print yields:

2024-01-01 you bought foo
    Assets:Test    10 FOO @@ $1000
    Assets:Test             $-1000

2024-01-01 you sold foo
    Assets:Test    10 FOO @@ $-1000
    Assets:Test               $1000

In fact this seems to work with per-unit costs as well - changing the rules file to:

fields date, description, symbol, quantity, price_per_share, amount

account1 Assets:Test
account2 Assets:Test
amount1 %quantity %symbol @ -$%price_per_share
amount2 $%amount

Yields

2024-01-01 you bought foo
    Assets:Test    10 FOO @ $100
    Assets:Test           $-1000

2024-01-01 you sold foo
    Assets:Test    10 FOO @ $-100
    Assets:Test             $1000

So the cost amounts are getting correctly negated here - including negation of negative values.

This seems to be a corner case due to my implicit usage of $ as the currency - when I remove $ from the rules file, I get the same parse error as above.

So I'm guessing something about parsing the currency here changes the behavior.

KyleBarton commented 3 months ago

But has this come up in real world data ?

For me it's helpful because of my usage of --strict, which I like because it ensures all of my accounts are declared. Otherwise for my purposes the following would work:

2024-01-01 you bought foo
    Assets:Test    10 FOO
    Assets:Test             $-1000

But doing so with --strict requires that I use --infer-equity, which creates equity:conversion accounts, which I don't really want to keep track of (but would have to due to my use of --strict). So it's really a convenience thing for me, FWIW. However, the behavior in my above comment seems inconsistent regardless.