Skip to content

Commit e17ef86

Browse files
committed
lib: use a transaction's amount precisions when balancing it
Surprising developments in old behaviour, as a consequence of #931: now that print shows amounts with all of their decimal places, we had better balance transactions using all of those visible digits (so that hledger and a user will agree on whether it's balanced). So now when transaction balancing compares amounts to see if they look equal, it uses (for each commodity) the maximum precision seen in just that transaction's amounts - not the precision from the journal's commodity display styles. This makes it more localised, which is a nice simplification. In journalFinalise, applying commodity display styles to the journal's amounts is now done as a final step (after transaction balancing, not before), and only once (rather than twice when auto postings are enabled), and seems slightly more thorough (affecting some inferred amounts where it didn't before). Inferred unit transaction prices (which arise in a two-commodity transaction with 3+ postings, and can be seen with print -x) may now be generated with a different number of decimal places than before. Specifically, it will be the sum of the the number of decimal places in the amounts being converted to and from. (Whereas before, it was.. some other, perhaps larger number.) Hopefully this will always be a suitable number of digits such that hledger's & users' calculation of balancedness will agree. Lib changes: Hledger.Data.Journal added: journalInferCommodityStyles journalInferAndApplyCommodityStyles removed: canonicalStyleFrom
1 parent 2fa60bb commit e17ef86

File tree

7 files changed

+192
-157
lines changed

7 files changed

+192
-157
lines changed

hledger-lib/Hledger/Data/Journal.hs

Lines changed: 66 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@ module Hledger.Data.Journal (
2323
addTransaction,
2424
journalBalanceTransactions,
2525
journalInferMarketPricesFromTransactions,
26+
journalInferCommodityStyles,
2627
journalApplyCommodityStyles,
27-
commodityStylesFromAmounts,
28+
journalInferAndApplyCommodityStyles,
2829
journalCommodityStyles,
2930
journalToCost,
3031
journalReverse,
@@ -78,7 +79,6 @@ module Hledger.Data.Journal (
7879
journalEquityAccountQuery,
7980
journalCashAccountQuery,
8081
-- * Misc
81-
canonicalStyleFrom,
8282
nulljournal,
8383
journalCheckBalanceAssertions,
8484
journalNumberAndTieTransactions,
@@ -87,7 +87,7 @@ module Hledger.Data.Journal (
8787
journalApplyAliases,
8888
-- * Tests
8989
samplejournal,
90-
tests_Journal,
90+
tests_Journal
9191
)
9292
where
9393

@@ -101,7 +101,7 @@ import Data.Function ((&))
101101
import qualified Data.HashTable.Class as H (toList)
102102
import qualified Data.HashTable.ST.Cuckoo as H
103103
import Data.List (find, sortOn)
104-
import Data.List.Extra (groupSort, nubSort)
104+
import Data.List.Extra (nubSort)
105105
import qualified Data.Map as M
106106
import Data.Maybe (catMaybes, fromJust, fromMaybe, isJust, mapMaybe)
107107
#if !(MIN_VERSION_base(4,11,0))
@@ -662,8 +662,7 @@ type Balancing s = ReaderT (BalancingState s) (ExceptT String (ST s))
662662
-- | The state used while balancing a sequence of transactions.
663663
data BalancingState s = BalancingState {
664664
-- read only
665-
bsStyles :: Maybe (M.Map CommoditySymbol AmountStyle) -- ^ commodity display styles
666-
,bsUnassignable :: S.Set AccountName -- ^ accounts in which balance assignments may not be used
665+
bsUnassignable :: S.Set AccountName -- ^ accounts in which balance assignments may not be used
667666
,bsAssrt :: Bool -- ^ whether to check balance assertions
668667
-- mutable
669668
,bsBalances :: H.HashTable s AccountName MixedAmount -- ^ running account balances, initially empty
@@ -722,18 +721,18 @@ updateTransactionB t = withRunningBalance $ \BalancingState{bsTransactions} ->
722721
-- and (optional) all balance assertions pass. Or return an error message
723722
-- (just the first error encountered).
724723
--
725-
-- Assumes journalInferCommodityStyles has been called, since those
726-
-- affect transaction balancing.
724+
-- Assumes the journal amounts' display styles still have the original number
725+
-- of decimal places that was parsed (ie, display styles have not yet been normalised),
726+
-- since this affects transaction balancing.
727727
--
728728
-- This does multiple things at once because amount inferring, balance
729729
-- assignments, balance assertions and posting dates are interdependent.
730+
--
730731
journalBalanceTransactions :: Bool -> Journal -> Either String Journal
731732
journalBalanceTransactions assrt j' =
732733
let
733734
-- ensure transactions are numbered, so we can store them by number
734735
j@Journal{jtxns=ts} = journalNumberTransactions j'
735-
-- display precisions used in balanced checking
736-
styles = Just $ journalCommodityStyles j
737736
-- balance assignments will not be allowed on these
738737
txnmodifieraccts = S.fromList $ map paccount $ concatMap tmpostingrules $ jtxnmodifiers j
739738
in
@@ -750,7 +749,7 @@ journalBalanceTransactions assrt j' =
750749
-- and leaving the others for later. The balanced ones are split into their postings.
751750
-- The postings and not-yet-balanced transactions remain in the same relative order.
752751
psandts :: [Either Posting Transaction] <- fmap concat $ forM ts $ \case
753-
t | null $ assignmentPostings t -> case balanceTransaction styles t of
752+
t | null $ assignmentPostings t -> case balanceTransaction t of
754753
Left e -> throwError e
755754
Right t' -> do
756755
lift $ writeArray balancedtxns (tindex t') t'
@@ -760,7 +759,7 @@ journalBalanceTransactions assrt j' =
760759
-- 2. Sort these items by date, preserving the order of same-day items,
761760
-- and step through them while keeping running account balances,
762761
runningbals <- lift $ H.newSized (length $ journalAccountNamesUsed j)
763-
flip runReaderT (BalancingState styles txnmodifieraccts assrt runningbals balancedtxns) $ do
762+
flip runReaderT (BalancingState txnmodifieraccts assrt runningbals balancedtxns) $ do
764763
-- performing balance assignments in, and balancing, the remaining transactions,
765764
-- and checking balance assertions as each posting is processed.
766765
void $ mapM' balanceTransactionAndCheckAssertionsB $ sortOn (either postingDate tdate) psandts
@@ -788,8 +787,7 @@ balanceTransactionAndCheckAssertionsB (Right t@Transaction{tpostings=ps}) = do
788787
-- update the account's running balance and check the balance assertion if any
789788
ps' <- forM ps $ \p -> pure (removePrices p) >>= addOrAssignAmountAndCheckAssertionB
790789
-- infer any remaining missing amounts, and make sure the transaction is now fully balanced
791-
styles <- R.reader bsStyles
792-
case balanceTransactionHelper styles t{tpostings=ps'} of
790+
case balanceTransactionHelper t{tpostings=ps'} of
793791
Left err -> throwError err
794792
Right (t', inferredacctsandamts) -> do
795793
-- for each amount just inferred, update the running balance
@@ -965,25 +963,66 @@ checkBalanceAssignmentUnassignableAccountB p = do
965963

966964
--
967965

966+
-- | Get an ordered list of amounts in this journal which can
967+
-- influence canonical amount display styles. Those are, in
968+
-- the following order:
969+
--
970+
-- * amounts in market price (P) directives (in parse order)
971+
-- * posting amounts in transactions (in parse order)
972+
-- * the amount in the final default commodity (D) directive
973+
--
974+
-- Transaction price amounts (posting amounts' aprice field) are not included.
975+
--
976+
journalStyleInfluencingAmounts :: Journal -> [Amount]
977+
journalStyleInfluencingAmounts j =
978+
dbg7 "journalStyleInfluencingAmounts" $
979+
catMaybes $ concat [
980+
[mdefaultcommodityamt]
981+
,map (Just . pdamount) $ jpricedirectives j
982+
,map Just $ concatMap amounts $ map pamount $ journalPostings j
983+
]
984+
where
985+
-- D's amount style isn't actually stored as an amount, make it into one
986+
mdefaultcommodityamt =
987+
case jparsedefaultcommodity j of
988+
Just (symbol,style) -> Just nullamt{acommodity=symbol,astyle=style}
989+
Nothing -> Nothing
990+
991+
-- | Infer commodity display styles for each commodity (see commodityStylesFromAmounts)
992+
-- based on the amounts in this journal (see journalStyleInfluencingAmounts),
993+
-- and save those inferred styles in the journal.
994+
-- Can return an error message eg if inconsistent number formats are found.
995+
journalInferCommodityStyles :: Journal -> Either String Journal
996+
journalInferCommodityStyles j =
997+
case commodityStylesFromAmounts $ journalStyleInfluencingAmounts j of
998+
Left e -> Left e
999+
Right cs -> Right j{jinferredcommodities = dbg7 "journalInferCommodityStyles" cs}
1000+
1001+
-- | Apply the given commodity display styles to the posting amounts in this journal.
1002+
journalApplyCommodityStyles :: M.Map CommoditySymbol AmountStyle -> Journal -> Journal
1003+
journalApplyCommodityStyles styles j@Journal{jtxns=ts, jpricedirectives=pds} =
1004+
j {jtxns=map fixtransaction ts
1005+
,jpricedirectives=map fixpricedirective pds
1006+
}
1007+
where
1008+
fixtransaction t@Transaction{tpostings=ps} = t{tpostings=map fixposting ps}
1009+
fixposting p = p{pamount=styleMixedAmount styles $ pamount p
1010+
,pbalanceassertion=fixbalanceassertion <$> pbalanceassertion p}
1011+
-- balance assertion/assignment amounts, and price amounts, are always displayed
1012+
-- (eg by print) at full precision
1013+
fixbalanceassertion ba = ba{baamount=styleAmountExceptPrecision styles $ baamount ba}
1014+
fixpricedirective pd@PriceDirective{pdamount=a} = pd{pdamount=styleAmountExceptPrecision styles a}
1015+
9681016
-- | Choose and apply a consistent display style to the posting
9691017
-- amounts in each commodity (see journalCommodityStyles).
9701018
-- Can return an error message eg if inconsistent number formats are found.
971-
journalApplyCommodityStyles :: Journal -> Either String Journal
972-
journalApplyCommodityStyles j@Journal{jtxns=ts, jpricedirectives=pds} =
1019+
journalInferAndApplyCommodityStyles :: Journal -> Either String Journal
1020+
journalInferAndApplyCommodityStyles j =
9731021
case journalInferCommodityStyles j of
9741022
Left e -> Left e
975-
Right j' -> Right j''
1023+
Right j' -> Right $ journalApplyCommodityStyles allstyles j'
9761024
where
977-
styles = journalCommodityStyles j'
978-
j'' = j'{jtxns=map fixtransaction ts
979-
,jpricedirectives=map fixpricedirective pds
980-
}
981-
fixtransaction t@Transaction{tpostings=ps} = t{tpostings=map fixposting ps}
982-
fixposting p = p{pamount=styleMixedAmount styles $ pamount p
983-
,pbalanceassertion=fixbalanceassertion <$> pbalanceassertion p}
984-
-- balance assertion amounts are always displayed (by print) at full precision, per docs
985-
fixbalanceassertion ba = ba{baamount=styleAmountExceptPrecision styles $ baamount ba}
986-
fixpricedirective pd@PriceDirective{pdamount=a} = pd{pdamount=styleAmountExceptPrecision styles a}
1025+
allstyles = journalCommodityStyles j'
9871026

9881027
-- | Get the canonical amount styles for this journal, whether (in order of precedence):
9891028
-- set globally in InputOpts,
@@ -1002,18 +1041,6 @@ journalCommodityStyles j =
10021041
defaultcommoditystyle = M.fromList $ catMaybes [jparsedefaultcommodity j]
10031042
inferredstyles = jinferredcommodities j
10041043

1005-
-- | Collect and save inferred amount styles for each commodity based on
1006-
-- the posting amounts in that commodity (excluding price amounts), ie:
1007-
-- "the format of the first amount, adjusted to the highest precision of all amounts".
1008-
-- Can return an error message eg if inconsistent number formats are found.
1009-
journalInferCommodityStyles :: Journal -> Either String Journal
1010-
journalInferCommodityStyles j =
1011-
case
1012-
commodityStylesFromAmounts $ journalStyleInfluencingAmounts j
1013-
of
1014-
Left e -> Left e
1015-
Right cs -> Right j{jinferredcommodities = dbg7 "journalInferCommodityStyles" cs}
1016-
10171044
-- -- | Apply this journal's historical price records to unpriced amounts where possible.
10181045
-- journalApplyPriceDirectives :: Journal -> Journal
10191046
-- journalApplyPriceDirectives j@Journal{jtxns=ts} = j{jtxns=map fixtransaction ts}

0 commit comments

Comments
 (0)