From 686a0871a9caac9d98cd624e820d995e7f1e035b Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Thu, 15 Apr 2021 11:58:30 +1000 Subject: [PATCH 1/2] lib: Make sure fromamount and toamount have opposite signs when inferring prices. Also change priceInferrerFor so that it would give the correct (negative) prices when fromamount and toamount have the same sign. --- hledger-lib/Hledger/Data/Transaction.hs | 41 +++++++++++++------------ hledger/test/journal/parse-errors.test | 2 +- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/hledger-lib/Hledger/Data/Transaction.hs b/hledger-lib/Hledger/Data/Transaction.hs index 54e9d93c5..ff39178fa 100644 --- a/hledger-lib/Hledger/Data/Transaction.hs +++ b/hledger-lib/Hledger/Data/Transaction.hs @@ -552,41 +552,42 @@ inferBalancingPrices t@Transaction{tpostings=ps} = t{tpostings=ps'} -- posting type (real or balanced virtual). If we cannot or should not infer -- prices, just act as the identity on postings. priceInferrerFor :: Transaction -> PostingType -> (Posting -> Posting) -priceInferrerFor t pt = maybe id inferprice $ inferFromAndTo sumamounts +priceInferrerFor t pt = maybe id inferprice inferFromAndTo where postings = filter ((==pt).ptype) $ tpostings t pcommodities = map acommodity $ concatMap (amounts . pamount) postings sumamounts = amounts $ sumPostings postings -- amounts normalises to one amount per commodity & price - noprices = all (isNothing . aprice) sumamounts - -- We can infer prices if there are no prices given, and exactly two commodities in the - -- normalised sum of postings in this transaction. The amount we are converting from is - -- the first commodity to appear in the ordered list of postings, and the commodity we - -- are converting to is the other. If we cannot infer prices, return Nothing. - inferFromAndTo [a,b] | noprices = asum $ map orderIfMatches pcommodities - where orderIfMatches x | x == acommodity a = Just (a,b) - | x == acommodity b = Just (b,a) - | otherwise = Nothing - inferFromAndTo _ = Nothing + -- We can infer prices if there are no prices given, exactly two commodities in the normalised + -- sum of postings in this transaction, and these two have opposite signs. The amount we are + -- converting from is the first commodity to appear in the ordered list of postings, and the + -- commodity we are converting to is the other. If we cannot infer prices, return Nothing. + inferFromAndTo = case sumamounts of + [a,b] | noprices, oppositesigns -> asum $ map orderIfMatches pcommodities + where + noprices = all (isNothing . aprice) sumamounts + oppositesigns = signum (aquantity a) /= signum (aquantity b) + orderIfMatches x | x == acommodity a = Just (a,b) + | x == acommodity b = Just (b,a) + | otherwise = Nothing + _ -> Nothing -- For each posting, if the posting type matches, there is only a single amount in the posting, -- and the commodity of the amount matches the amount we're converting from, -- then set its price based on the ratio between fromamount and toamount. inferprice (fromamount, toamount) posting | [a] <- amounts (pamount posting), ptype posting == pt, acommodity a == acommodity fromamount - , let totalpricesign = if aquantity a < 0 then negate else id - = posting{ pamount = mixedAmount a{aprice=Just $ conversionprice totalpricesign} + = posting{ pamount = mixedAmount a{aprice=Just conversionprice} , poriginal = Just $ originalPosting posting } | otherwise = posting where - -- If only one Amount in the posting list matches fromamount we can use TotalPrice, - -- but we need to know the sign. Otherwise divide the conversion equally among the - -- Amounts by using a unit price. - conversionprice sign = case filter (== acommodity fromamount) pcommodities of - [_] -> TotalPrice $ sign (abs toamount) `withPrecision` NaturalPrecision - _ -> UnitPrice $ abs unitprice `withPrecision` unitprecision + -- If only one Amount in the posting list matches fromamount we can use TotalPrice. + -- Otherwise divide the conversion equally among the Amounts by using a unit price. + conversionprice = case filter (== acommodity fromamount) pcommodities of + [_] -> TotalPrice $ negate toamount `withPrecision` NaturalPrecision + _ -> UnitPrice $ negate unitprice `withPrecision` unitprecision - unitprice = (aquantity fromamount) `divideAmount` toamount + unitprice = aquantity fromamount `divideAmount` toamount unitprecision = case (asprecision $ astyle fromamount, asprecision $ astyle toamount) of (Precision a, Precision b) -> Precision . max 2 $ saturatedAdd a b _ -> NaturalPrecision diff --git a/hledger/test/journal/parse-errors.test b/hledger/test/journal/parse-errors.test index 51eb84294..f210edf08 100644 --- a/hledger/test/journal/parse-errors.test +++ b/hledger/test/journal/parse-errors.test @@ -127,7 +127,7 @@ hledger: "-" (lines 1-3) could not balance this transaction: real postings all have the same sign 2020-01-01 - a 1A @@ 1B + a 1A b 1B >=1 From d1467d4766102917905961ca9b8e1994804715f3 Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Fri, 16 Apr 2021 10:53:52 +1000 Subject: [PATCH 2/2] lib: When inferring total prices, use the precision of toamount, rather than NaturalPrecision. --- hledger-lib/Hledger/Data/Transaction.hs | 4 ++-- hledger/test/journal/auto-postings.test | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/hledger-lib/Hledger/Data/Transaction.hs b/hledger-lib/Hledger/Data/Transaction.hs index ff39178fa..16f0b9fe6 100644 --- a/hledger-lib/Hledger/Data/Transaction.hs +++ b/hledger-lib/Hledger/Data/Transaction.hs @@ -584,7 +584,7 @@ priceInferrerFor t pt = maybe id inferprice inferFromAndTo -- If only one Amount in the posting list matches fromamount we can use TotalPrice. -- Otherwise divide the conversion equally among the Amounts by using a unit price. conversionprice = case filter (== acommodity fromamount) pcommodities of - [_] -> TotalPrice $ negate toamount `withPrecision` NaturalPrecision + [_] -> TotalPrice $ negate toamount _ -> UnitPrice $ negate unitprice `withPrecision` unitprecision unitprice = aquantity fromamount `divideAmount` toamount @@ -918,7 +918,7 @@ tests_Transaction = [ posting {paccount = "a", pamount = mixedAmount (usd 1.35)} , posting {paccount = "b", pamount = mixedAmount (eur (-1))} ])) @?= - Right (mixedAmount $ usd 1.35 @@ (eur 1 `withPrecision` NaturalPrecision)) + Right (mixedAmount $ usd 1.35 @@ eur 1) ,test "balanceTransaction balances based on cost if there are unit prices" $ assertRight $ balanceTransaction diff --git a/hledger/test/journal/auto-postings.test b/hledger/test/journal/auto-postings.test index f8cb27aaf..000bfccfe 100644 --- a/hledger/test/journal/auto-postings.test +++ b/hledger/test/journal/auto-postings.test @@ -230,11 +230,11 @@ $ hledger -f- print --auto # 12. $ hledger -f- print --auto 2018-01-01 ; modified: - Expenses:Joint:Widgets $100.00 - Expenses:Joint $-100.00 @@ £50 ; generated-posting: = ^Expenses:Joint - Liabilities:Joint:Bob $50.00 @@ £25 ; generated-posting: = ^Expenses:Joint - Liabilities:Joint:Bill $50.00 @@ £25 ; generated-posting: = ^Expenses:Joint - Assets:Joint:Bank £-50.00 + Expenses:Joint:Widgets $100.00 + Expenses:Joint $-100.00 @@ £50.00 ; generated-posting: = ^Expenses:Joint + Liabilities:Joint:Bob $50.00 @@ £25.00 ; generated-posting: = ^Expenses:Joint + Liabilities:Joint:Bill $50.00 @@ £25.00 ; generated-posting: = ^Expenses:Joint + Assets:Joint:Bank £-50.00 >=0