slaesvuo/hledger
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

;roi: more input sanity checks (fixes #2505)

Browse Source
This commit is contained in:
Dmitry Astapov 2025-12-19 23:18:33 +00:00 committed by Simon Michael
parent 2fb09f0d1e
commit 1c37dc44a7
2 changed files with 25 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
hledger/Hledger/Cli/Commands/Roi.hs
View File

@ -23,6 +23,7 @@ import Data.Function (on)
import Data.List import Data.List
import Numeric.RootFinding import Numeric.RootFinding
import Data.Decimal import Data.Decimal
import Data.Set qualified as S
import Data.Text qualified as T import Data.Text qualified as T
import Data.Text.Lazy.IO qualified as TL import Data.Text.Lazy.IO qualified as TL
import Safe (headDef) import Safe (headDef)
@ -83,10 +84,14 @@ roi CliOpts{rawopts_=rawopts, reportspec_=rspec@ReportSpec{_rsReportOpts=ReportO
makeQuery flag = do makeQuery flag = do
q <- either usageError (return . fst) . parseQuery today . T.pack $ stringopt flag rawopts q <- either usageError (return . fst) . parseQuery today . T.pack $ stringopt flag rawopts
return . simplifyQuery $ And [queryFromFlags ropts{period_=PeriodAll}, q] return . simplifyQuery $ And [queryFromFlags ropts{period_=PeriodAll}, q]
cantCompute msg = error' $ msg ++ " - will be unable to compute the rates of return"
investmentsQuery <- makeQuery "investment" investmentsQuery <- makeQuery "investment"
pnlQuery <- makeQuery "pnl" pnlQuery <- makeQuery "pnl"
when (pnlQuery == Any) $
cantCompute "Need some transactions classed as investment and not pnl, but the pnl query matches any transaction"
let let
filteredj = filterJournalTransactions investmentsQuery j filteredj = filterJournalTransactions investmentsQuery j
trans = dbg3 "investments" $ jtxns filteredj trans = dbg3 "investments" $ jtxns filteredj
@ -96,7 +101,7 @@ roi CliOpts{rawopts_=rawopts, reportspec_=rspec@ReportSpec{_rsReportOpts=ReportO
let (fullPeriodDateSpan, mspans) = reportSpan filteredj rspec let (fullPeriodDateSpan, mspans) = reportSpan filteredj rspec
let err = error' "Undefined start or end of the period - will be unable to compute the rates of return" let err = cantCompute "Undefined start or end of the period"
spans = maybe err (map (second (addDays 1)) . dayPartitionToList) mspans spans = maybe err (map (second (addDays 1)) . dayPartitionToList) mspans
fullPeriod = case fullPeriodDateSpan of fullPeriod = case fullPeriodDateSpan of
DateSpan (Just b) (Just e) -> (fromEFDay b, fromEFDay e) DateSpan (Just b) (Just e) -> (fromEFDay b, fromEFDay e)
@ -125,15 +130,21 @@ roi CliOpts{rawopts_=rawopts, reportspec_=rspec@ReportSpec{_rsReportOpts=ReportO
, Not pnlQuery , Not pnlQuery
, Date spn ] ) , Date spn ] )
pnl = pnl = dbg3 "pnl" $
cashFlowApplyCostValue $ cashFlowApplyCostValue $
calculateCashFlow wd trans (And [ Not investmentsQuery calculateCashFlow wd trans (And [ Not investmentsQuery
, pnlQuery , pnlQuery
, Date spn ] ) , Date spn ] )
spanCommodities = dbg3 "spanCommodities" $
S.unions $ (maCommodities valueBefore) : (maCommodities valueAfter) : map (maCommodities.snd) (cashFlow ++ pnl)
thisSpan = dbg3 "processing span" $ thisSpan = dbg3 "processing span" $
OneSpan b e valueBefore valueAfter cashFlow pnl OneSpan b e valueBefore valueAfter cashFlow pnl
when (S.size spanCommodities > 1) $
multiCommodityError $ "Period " ++ show (b,e) ++ " has multiple commodities: " ++ (T.unpack $ T.intercalate ", " $ map showCommoditySymbol (S.toList spanCommodities))
irr <- internalRateOfReturn styles showCashFlow prettyTables thisSpan irr <- internalRateOfReturn styles showCashFlow prettyTables thisSpan
(periodTwr, annualizedTwr) <- timeWeightedReturn styles showCashFlow prettyTables investmentsQuery trans mixedAmountValue thisSpan (periodTwr, annualizedTwr) <- timeWeightedReturn styles showCashFlow prettyTables investmentsQuery trans mixedAmountValue thisSpan
let cashFlowAmt = maNegate . maSum $ map snd cashFlow let cashFlowAmt = maNegate . maSum $ map snd cashFlow
@ -194,7 +205,7 @@ timeWeightedReturn _styles showCashFlow prettyTables investmentsQuery trans mixe
valueBefore = dbg3 ("value at the start of the interval, "++show begin) $ unMix valueBeforeAmt valueBefore = dbg3 ("value at the start of the interval, "++show begin) $ unMix valueBeforeAmt
valueAfter = dbg3 ("value at the end of the interval, "++show end) $ unMix valueAfterAmt valueAfter = dbg3 ("value at the end of the interval, "++show end) $ unMix valueAfterAmt
investmentPostings = concatMap (filter (matchesPosting investmentsQuery) . realPostings) trans investmentPostings = concatMap (filter (matchesPosting investmentsQuery) . realPostings) trans
totalInvestmentPostingsTill date = sumPostings $ filter (matchesPosting (Date (DateSpan Nothing (Just $ Exact date)))) investmentPostings totalInvestmentPostingsTill date = sumPostings $ filter (matchesPosting (Date (DateSpan Nothing (Just $ Exact date)))) investmentPostings
@ -207,7 +218,7 @@ timeWeightedReturn _styles showCashFlow prettyTables investmentsQuery trans mixe
-- We are dividing the period [begin, end) into subperiods on each cashflow, and then compute -- We are dividing the period [begin, end) into subperiods on each cashflow, and then compute
-- the rate of return for each subperiod. For this we need to know the value of the investment -- the rate of return for each subperiod. For this we need to know the value of the investment
-- at the beginning and end of each subperiod, adjusted for cashflow. -- at the beginning and end of each subperiod, adjusted for cashflow.
-- --
-- Subperiods are going to be [valueBefore ... (c_0,v_0)][... (c_1, v_1)][... (c_2,v_2)] ... [... (c_n,v_n)][... valueAfter] -- Subperiods are going to be [valueBefore ... (c_0,v_0)][... (c_1, v_1)][... (c_2,v_2)] ... [... (c_n,v_n)][... valueAfter]
-- , where v_i is the value of investment computed immediately after cashflow c_i -- , where v_i is the value of investment computed immediately after cashflow c_i
-- --
@ -318,9 +329,11 @@ unMix :: MixedAmount -> Quantity
unMix a = unMix a =
case (unifyMixedAmount a) of case (unifyMixedAmount a) of
Just a' -> aquantity a' Just a' -> aquantity a'
Nothing -> error' $ "Amounts could not be converted to a single commodity: " ++ show (map showAmount $ amounts a) ++ Nothing -> multiCommodityError $ "Amounts could not be converted to a single commodity: " ++ show (map showAmount $ amounts a)
"\nConsider using --value to force all costs to be in a single commodity." ++
"\nFor example, \"--value=end,<commodity> --infer-market-prices\", where commodity is the one that was used for investment valuations." multiCommodityError msg = error' $ msg ++
"\nConsider using --value to force all costs to be in a single commodity." ++
"\nFor example, \"--value=end,<commodity> --infer-market-prices\", where commodity is the one that you want to value the investment in."
-- Show Decimal rounded to two decimal places, unless it has less places already. This ensures that "2" won't be shown as "2.00" -- Show Decimal rounded to two decimal places, unless it has less places already. This ensures that "2" won't be shown as "2.00"
showDecimal :: Decimal -> String showDecimal :: Decimal -> String

7
hledger/test/roi.test
View File

@ -212,7 +212,7 @@ $ hledger -f- roi --inv investment --pnl pnl -b 2017-06 -e 2018
2019/11/01 Example 2019/11/01 Example
Assets:Checking 1 Assets:Checking 1
Income:Salary -1 Income:Salary -1
$ hledger -f- roi -p 2019-11 $ hledger -f- roi -p 2019-11 --pnl "^$"
+---++------------+------------++---------------+----------+-------------+-----++-------++------------+----------+ +---++------------+------------++---------------+----------+-------------+-----++-------++------------+----------+
| || Begin | End || Value (begin) | Cashflow | Value (end) | PnL || IRR || TWR/period | TWR/year | | || Begin | End || Value (begin) | Cashflow | Value (end) | PnL || IRR || TWR/period | TWR/year |
+===++============+============++===============+==========+=============+=====++=======++============+==========+ +===++============+============++===============+==========+=============+=====++=======++============+==========+
@ -236,7 +236,10 @@ $ hledger -f- roi -p 2019-11
Assets:Checking 101 A Assets:Checking 101 A
Unrealized PnL Unrealized PnL
$ hledger -f- roi -p 2019-11 --inv Investment --pnl PnL $ hledger -f- roi -p 2019-11 --inv Investment --pnl PnL
>2 /Error: Amounts could not be converted to a single commodity: \["10 B","-9 B @@ 100 A","100 C"\]/ >2
hledger: Error: Period (2019-11-01,2019-12-01) has multiple commodities: A, B, C
Consider using --value to force all costs to be in a single commodity.
For example, "--value=end,<commodity> --infer-market-prices", where commodity is the one that you want to value the investment in.
>= 1 >= 1
# ** 10. Forcing valuation via --value # ** 10. Forcing valuation via --value