From 8d75635505beb2b1b8a95c9b5d25e191cab27be4 Mon Sep 17 00:00:00 2001 From: Simon Michael Date: Wed, 27 May 2015 14:21:19 -0700 Subject: [PATCH] print: limit display precision of generated prices (#262) When a transaction posts to two commodities without specifying the conversion price, we generate a price which makes it balance (cf http://hledger.org/manual.html#prices). Until now, these generated prices were always shown with full precision (all available decimal digits) so that a manual calculation with the displayed numbers would agree. If there's just one posting in the commodity being priced, we can use an exact total price and the precision is no problem. But if there are multiple postings in the commodity being priced, we must show the averaged unit price. This can be an irrational number, which with our current Decimal-based implementation would display an excessive 255 decimal digits. So in this case we now set the price's display precision to the sum of the (max) display precisions of the commodities involved. An example: hledgerdev -f- print <<< 1/1 c C 10.00 c C 11.00 d D -320.00 >>> 2015/01/01 c C 10.00 @ D 15.2381 c C 11.00 @ D 15.2381 d D -320.00 >>>=0 There might still be cases where this will show more price decimal places than necessary. For now, YAGNI. --- hledger-lib/Hledger/Data/Transaction.hs | 36 ++++++++++++++++------ tests/misc/precision.test | 40 ++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 10 deletions(-) diff --git a/hledger-lib/Hledger/Data/Transaction.hs b/hledger-lib/Hledger/Data/Transaction.hs index 8203a34a9..8980e34a9 100644 --- a/hledger-lib/Hledger/Data/Transaction.hs +++ b/hledger-lib/Hledger/Data/Transaction.hs @@ -260,6 +260,7 @@ isTransactionBalanced styles t = bvsum' = canonicalise $ costOfMixedAmount bvsum canonicalise = maybe id canonicaliseMixedAmount styles +-- XXX refactor -- | Ensure this transaction is balanced, possibly inferring a missing -- amount or conversion price, or return an error message. -- @@ -298,30 +299,45 @@ balanceTransaction styles t@Transaction{tpostings=ps} ramountsinorder = concatMap amounts rmixedamountsinorder rcommoditiesinorder = map acommodity ramountsinorder rsumamounts = amounts $ sum rmixedamountsinorder - -- assumption: the sum of mixed amounts is normalised (one simple amount per commodity) - t'' = if length rsumamounts == 2 && all ((==NoPrice).aprice) rsumamounts && t'==t + -- as it says above, we can infer a conversion price when + t'' = if t'==t -- all real amounts were explicit (we didn't have to infer any) + && length rsumamounts == 2 -- and the sum of real amounts has exactly two commodities (assumption: summing mixed amounts normalises to one simple amount per commodity) + && all ((==NoPrice).aprice) rsumamounts -- and none of the amounts had explicit prices then t'{tpostings=map inferprice ps} else t' where - -- assumption: a posting's mixed amount contains one simple amount - inferprice p@Posting{pamount=Mixed [a@Amount{acommodity=c,aprice=NoPrice}], ptype=RegularPosting} + inferprice p@Posting{pamount=Mixed [a@Amount{acommodity=c,aprice=NoPrice}], ptype=RegularPosting} -- assumption: a posting's mixed amount contains one simple amount = p{pamount=Mixed [a{aprice=conversionprice c}]} where conversionprice c | c == unpricedcommodity - -- assign a balancing price. Use @@ for more exact output when possible. - -- invariant: prices should always be positive. Enforced with "abs" + + -- calculate a price that makes the postings balance, and give it "just enough" + -- display precision that a manual calculation with the displayed numbers + -- shows the transaction balancing. = if length ramountsinunpricedcommodity == 1 + + -- when there is only one posting in the target commodity, + -- show a total price (@@) for more exact output. In this + -- case show all available decimal digits, it shouldn't be too many. then TotalPrice $ abs targetcommodityamount `withPrecision` maxprecision - else UnitPrice $ abs (targetcommodityamount `divideAmount` (aquantity unpricedamount)) `withPrecision` maxprecision + + -- otherwise, calculate the average unit conversion price across all postings. + -- Set the precision to the sum of the precisions of the commodities involved, + -- which should be enough to make calculation look right while also preventing + -- irrational numbers from printing excessive digits. + else UnitPrice $ abs unitprice `withPrecision` sumofprecisions + | otherwise = NoPrice where unpricedcommodity = head $ filter (`elem` (map acommodity rsumamounts)) rcommoditiesinorder unpricedamount = head $ filter ((==unpricedcommodity).acommodity) rsumamounts targetcommodityamount = head $ filter ((/=unpricedcommodity).acommodity) rsumamounts ramountsinunpricedcommodity = filter ((==unpricedcommodity).acommodity) ramountsinorder + unitprice = targetcommodityamount `divideAmount` (aquantity unpricedamount) + sumofprecisions = (asprecision $ astyle $ targetcommodityamount) + (asprecision $ astyle $ unpricedamount) inferprice p = p - -- maybe infer prices for balanced virtual postings. Just duplicates the above for now. + -- maybe infer prices for balanced virtual postings. Duplicates the above. XXX bvmixedamountsinorder = map pamount $ balancedVirtualPostings t'' bvamountsinorder = concatMap amounts bvmixedamountsinorder bvcommoditiesinorder = map acommodity bvamountsinorder @@ -336,13 +352,15 @@ balanceTransaction styles t@Transaction{tpostings=ps} conversionprice c | c == unpricedcommodity = if length bvamountsinunpricedcommodity == 1 then TotalPrice $ abs targetcommodityamount `withPrecision` maxprecision - else UnitPrice $ abs (targetcommodityamount `divideAmount` (aquantity unpricedamount)) `withPrecision` maxprecision + else UnitPrice $ abs unitprice `withPrecision` sumofprecisions | otherwise = NoPrice where unpricedcommodity = head $ filter (`elem` (map acommodity bvsumamounts)) bvcommoditiesinorder unpricedamount = head $ filter ((==unpricedcommodity).acommodity) bvsumamounts targetcommodityamount = head $ filter ((/=unpricedcommodity).acommodity) bvsumamounts bvamountsinunpricedcommodity = filter ((==unpricedcommodity).acommodity) bvamountsinorder + unitprice = targetcommodityamount `divideAmount` (aquantity unpricedamount) + sumofprecisions = (asprecision $ astyle $ targetcommodityamount) + (asprecision $ astyle $ unpricedamount) inferprice p = p -- tie the knot so eg relatedPostings works right diff --git a/tests/misc/precision.test b/tests/misc/precision.test index 583290dcd..2cca53902 100644 --- a/tests/misc/precision.test +++ b/tests/misc/precision.test @@ -78,6 +78,7 @@ hledgerdev -f - balance --cost -------------------- 0 >>>=0 + # hledger 0.14pre: precision=2, presumably from price # $3266.32 assets:investment:ACME # $-3266.32 equity:opening balances @@ -93,8 +94,8 @@ hledgerdev -f - balance --cost # $-3266 equity:opening balances #-------------------- # 0 -## 6. with a default commodity.. XXX should observe it +## 6. with a default commodity.. XXX should observe it hledgerdev -f - balance --cost <<< D $1000.0 @@ -122,3 +123,40 @@ D $1000.0 ### $-3266.3 equity:opening balances ###-------------------- ### 0 + +# +# 7. when there are multiple postings in the commodity being priced, +# show the averaged unit price, with precision equal to the sum of +# the max precisions of the commodities being converted (#262). +# Here the (irrational) price should be displayed with just precision 4 +# (C's precision 2 + D's precision 2). +hledgerdev -f- print +<<< +1/1 + c C 10.00 + c C 11.00 + d D -320.00 +>>> +2015/01/01 + c C 10.00 @ D 15.2381 + c C 11.00 @ D 15.2381 + d D -320.00 + +>>>=0 + + +## 8. Here the price should be displayed with precision 7 +# (E's precision 4 + F's precision 3). +hledgerdev -f- print +<<< +1/1 + e E 10.0000 + e E 11 + f F -320.000 +>>> +2015/01/01 + e E 10.0000 @ F 15.2380952 + e E 11.0000 @ F 15.2380952 + f F -320.000 + +>>>=0