gnucash maint: gnc:get-commodity-totalavg-prices shouldn't use 0-amount splits.
John Ralls
jralls at code.gnucash.org
Thu Aug 30 14:05:14 EDT 2018
Updated via https://github.com/Gnucash/gnucash/commit/6dfbf3d5 (commit)
from https://github.com/Gnucash/gnucash/commit/c977c235 (commit)
commit 6dfbf3d5e7bed8e09c650b994e62ff74715bdfc9
Author: John Ralls <jralls at ceridwen.us>
Date: Thu Aug 30 11:02:16 2018 -0700
gnc:get-commodity-totalavg-prices shouldn't use 0-amount splits.
get-commodity-totalavg-prices seeks to create a share-weighted average of
all prices for a commodity and 0-share splits (normally used to book
trading gains) don't represent a price and so distort (sometimes
dramatically) the resulting average as well as creating extra invalid
entries in the resulting alist.
diff --git a/gnucash/report/report-system/commodity-utilities.scm b/gnucash/report/report-system/commodity-utilities.scm
index 93ceac8..b21e906 100644
--- a/gnucash/report/report-system/commodity-utilities.scm
+++ b/gnucash/report/report-system/commodity-utilities.scm
@@ -116,7 +116,8 @@
;; Returns true if the given pricealist element is a non-zero price.
(define (gnc:price-is-not-zero? elem)
- (not (gnc-numeric-zero-p (second elem))))
+ (and (second elem)
+ (not (gnc-numeric-zero-p (second elem)))))
;; Create a list of all prices of 'price-commodity' measured in the currency
;; 'report-currency'. The prices are taken from all splits in
@@ -155,12 +156,16 @@
(transaction-date (xaccTransGetDate
(xaccSplitGetParent a)))
(foreignlist
- (if (gnc-commodity-equiv transaction-comm
- price-commodity)
- (list account-comm
- share-amount value-amount)
- (list transaction-comm
- value-amount share-amount))))
+ (if (and
+ (not (zero? share-amount))
+ (not (zero? value-amount)))
+ (if (gnc-commodity-equiv transaction-comm
+ price-commodity)
+ (list account-comm
+ share-amount value-amount)
+ (list transaction-comm
+ value-amount share-amount))
+ #f)))
;;(warn "gnc:get-commodity-totalavg-prices: value "
;; (gnc-commodity-numeric->string
@@ -170,8 +175,9 @@
;;price-commodity (third foreignlist)))
;; Try EURO exchange if necessary
- (if (not (gnc-commodity-equiv (first foreignlist)
- report-currency))
+ (if (and foreignlist
+ (not (gnc-commodity-equiv (first foreignlist)
+ report-currency)))
(let ((exchanged (gnc:exchange-by-euro-numeric
(first foreignlist) (second foreignlist)
report-currency transaction-date)))
@@ -183,35 +189,37 @@
(list
transaction-date
- (if (not (gnc-commodity-equiv (first foreignlist)
- report-currency))
- (begin
- (warn "gnc:get-commodity-totalavg-prices: "
- "Sorry, currency exchange not yet implemented:"
- (gnc-commodity-numeric->string
- (first foreignlist) (second foreignlist))
- " (buying "
- (gnc-commodity-numeric->string
- price-commodity (third foreignlist))
- ") =? "
- (gnc-commodity-numeric->string
- report-currency (gnc-numeric-zero)))
- (gnc-numeric-zero))
- (begin
- (set! total-foreign (gnc-numeric-add total-foreign
- (third foreignlist)
- GNC-DENOM-AUTO
- GNC-DENOM-LCD))
- (set! total-domestic (gnc-numeric-add total-domestic
- (second foreignlist)
- GNC-DENOM-AUTO
- GNC-DENOM-LCD))
- (if (not (zero? total-foreign))
- (gnc-numeric-div
- total-domestic
- total-foreign
- GNC-DENOM-AUTO
- (logior (GNC-DENOM-SIGFIGS 8) GNC-RND-ROUND)) 0))))))
+ (if foreignlist
+ (if (not (gnc-commodity-equiv (first foreignlist)
+ report-currency))
+ (begin
+ (warn "gnc:get-commodity-totalavg-prices: "
+ "Sorry, currency exchange not yet implemented:"
+ (gnc-commodity-numeric->string
+ (first foreignlist) (second foreignlist))
+ " (buying "
+ (gnc-commodity-numeric->string
+ price-commodity (third foreignlist))
+ ") =? "
+ (gnc-commodity-numeric->string
+ report-currency (gnc-numeric-zero)))
+ (gnc-numeric-zero))
+ (begin
+ (set! total-foreign (gnc-numeric-add total-foreign
+ (third foreignlist)
+ GNC-DENOM-AUTO
+ GNC-DENOM-LCD))
+ (set! total-domestic (gnc-numeric-add total-domestic
+ (second foreignlist)
+ GNC-DENOM-AUTO
+ GNC-DENOM-LCD))
+ (if (not (zero? total-foreign))
+ (gnc-numeric-div
+ total-domestic
+ total-foreign
+ GNC-DENOM-AUTO
+ (logior (GNC-DENOM-SIGFIGS 8) GNC-RND-ROUND)) 0)))
+ #f))))
;; Get all the interesting splits, and sort them according to the
;; date.
(gnc:get-match-commodity-splits-sorted
@@ -985,6 +993,9 @@
(lambda (foreign domestic date)
(gnc:exchange-by-pricealist-nearest
pricealist foreign domestic date))))
+ ;; actual-transactions isn't used, at least not as a value passed to this
+ ;; function. price-scatter.scm does use it but calls
+ ;; gnc:get-commodity-inst-prices directly.
((actual-transactions) (let ((pricealist
(gnc:get-commoditylist-inst-prices
commodity-list report-currency to-date-tp)))
diff --git a/gnucash/report/report-system/test/test-commodity-utils.scm b/gnucash/report/report-system/test/test-commodity-utils.scm
index 1a03a21..728da7e 100644
--- a/gnucash/report/report-system/test/test-commodity-utils.scm
+++ b/gnucash/report/report-system/test/test-commodity-utils.scm
@@ -537,16 +537,16 @@
;; the comment at the gnc:get-commodity-totalavg-prices definition for more
;; about the prices from this function.
(test-equal "MSFT totalavg 2014-12-05"
- (gnc-numeric-div 5232000/100 1500 GNC-DENOM-AUTO
+ (gnc-numeric-div 6637500/100 2000 GNC-DENOM-AUTO
(logior (GNC-DENOM-SIGFIGS 8) GNC-RND-ROUND))
(cadr (assoc (gnc-dmy2time64-neutral 5 12 2014)
report-list)))
(test-equal "MSFT totalavg 2015-04-02"
- (gnc-numeric-div 10876200/100 2800 GNC-DENOM-AUTO
+ (gnc-numeric-div 9860700/100 2800 GNC-DENOM-AUTO
(logior (GNC-DENOM-SIGFIGS 8) GNC-RND-ROUND))
(cadr (assoc (gnc-dmy2time64-neutral 2 4 2015) report-list)))
(test-equal "MSFT totalavg 2016-03-11"
- (gnc-numeric-div 12634400/100 2800 GNC-DENOM-AUTO
+ (gnc-numeric-div 14637000/100 3700 GNC-DENOM-AUTO
(logior (GNC-DENOM-SIGFIGS 8) GNC-RND-ROUND))
(cadr (assoc (gnc-dmy2time64-neutral 11 3 2016)
report-list))))
Summary of changes:
.../report/report-system/commodity-utilities.scm | 87 ++++++++++++----------
.../report-system/test/test-commodity-utils.scm | 6 +-
2 files changed, 52 insertions(+), 41 deletions(-)
More information about the gnucash-changes
mailing list