Bug 798879 - RFE: [Transaction Report] add Running Total option

[dawansv]

Summary

Running total are calculated based on report transactions sort order as shown. Can be displayed on the main total as well as the primary and secondary subtotal levels. Compatible with multi-currency options and multi-line (splits) display.

Context

The idea came when I was trying to use the running balance feature to create running totals and realized it wasn't doing what I wanted (although it's a usefull feature in itself). What I wanted was a running total based on the transactions in the report in whatever order and filtering I had selected.

I started to work on in gnucash version 4. My code was improved based on useful (and very patient) feedback I received on a couple of previous PR (code in the first one was pretty bad, second one better) and is now presented with additional improvements for version 5. It is a work in progress requiring more pairs of eyes both in terms of code but also presentation. Because it changes trep-engine, care must also be taken to not impact other reports and custom reports based on it, so additional testing will be done there as well. In the meantime I am opening this draft PR to get additional feedback.

The Feature

The running totals work on all 3 levels: Grand Total, Primary Subtotal and Secondary Subtotal. They can be enabled or disabled separetely. In the screenshot below all 3 are enabled at once (with a small dataset to keep the screenshot manageable).

The options to enables them are right under each of their respective options (so in the Display section for the grand total and the Sorting section for the subtotals).

The running totals are calculated as per the grouping and ordering presented in the report, not the underlying data. It is for the user to decide whether a particular grouping and ordering is relevant (such as combining transactions from several accounts in the same list ordered by date). But the running totals will follow whatever grouping and sorting option the user chooses.

Here is an example of a running total calculated by combining transactions from both wallet and checking accounts mixed together with an unusual descending date sort order. This examples is meant to highlight that the relevance of the nature and order of transaction data presented is left to the user, the running total is not judgmental in that regard.

The feature is fully compliant with the multi currency display (including showing converted and original amounts, each with their own set of running total and subtotals columns).

Caveats

1. Although fully compliant with the multi currency display, in the multicurrency columns, only the running total of the current split’s currency is shown on each line, while all currency are shown in the final total at the bottom on the corresponding amount column (current behavior). Showing all currencies on each transaction line would not only require significant code change but also most likely clutter the report. Additionally, the total is not currently shown on the running total columns themselves as that would require significant additional changes to how subtotals are calculated and displayed. Minimizing the effect is the fact that running totals are only enables when their corresponding total or subtotal is enabled.
2. The running totals get their data from the splits, not the other calculated columns themselves. If one were to transform the data in a custom calculator-function for the amount columns, then of course the running total calculator-functions would need to be updated accordingly with their own custom calculator-functions. That is to be expected and fully supported by the trep-engine’s custom-calculated-cells feature.
3. The running totals suffer from the same issue as the subtotals as related to the “Sign Reverses” option. Just as they are for the subtotals, signs are not reversed for the running totals since they have to match the totals on the last transaction of their respective level. The issues around the “Sign Reverses” option have been discussed other places and if anything is ever decided as relating to either that option or changes to the subtotals then the same would be done to the running totals. However there is nothing that can be done here differently until that other issue is resolved.

Technical brief and code review

Although this introduces close to 200 lines of new code and a few lines of modified code, most additions are pretty benign boilerplate to create new options, new columns, etc. following existing code structures, and with plenty of comments to help with long-term maintainability.

There is only one major change (discussed in detail below) which calls for the introduction of a second argument in the calculated-cells calculator-functions (while preserving the support of calculator-functions with a single argument) with related code change in the add-split-row renderer.

A second smaller change was made to selecting which number columns are formatted with a link to the underlying transaction in the register is also discussed below (discussed further down).

A third small change is in removing columns that do not have subtotals enabled from calculated-cells when “Show subtotals only” is selected. This should have been done when that feature was developed, and the missing code caused empty columns to be displayed in that view (including the existing account balance column) an issue made more apparent with the new running total columns.

Changes to add-split-row and the related calculator-functions

Add-split-row is the procedure that is called to add each row of data to the report. It received a split and runs it through the calculator-function for each cell.

In the definition part of the report, the columns are divided in two groups:

1. the left-columns (date, description, notes, etc.), and
2. the calculated-cells (also referred as right-columns) that contain the numeric data calculated from the split amount or retrieved based on it (single amount, credit, debit, running balance, etc).

One key difference between the left-columns and calculated-cells is that the function that are called to retrieve the content of the left-columns are provided with a second parameter called transaction-row? which differentiates the splits in a multiline situation (when "Display – Detail Level – One Split per Line” is selected). The flag is set to #t when the current split is the “main” split related to the account and #f when the current split is another split. This allows the left-columns to only show the date and description when the main split is shown, and leaves that information blank when not (since it is the same for the splits).

Unlike the left-columns, the calculator-functions for the calculated-cells do not have that argument. They only get the split with no way to tell if it’s a main split or “other” split. This is a problem for the running totals in a multiline environment since they should only be computed and shown on the main split, not the other splits. In fact the same problem exists on the Running Balance which for now is disabled in the multiline context.

Hence my code changes implement this additional argument for the calculator-functions (calculator-function split transaction-row?) while preserving the ability to have calculator-functions with only one argument (calculator-function split) for legacy purpose.

This results in one of the main code additions and changes in add-split-row

             ;; Returns the number of required arguments of a function
             (fn-arity (lambda (fn) (car (procedure-minimum-arity fn))))
             ;; Calls the calculator-function for the provided cell with either
             ;; two arguments (calculator-function split transaction-row?) or only
             ;; one argument  (calculator-function split) for legacy support.
             (calc-fn (lambda (cell split transaction-row?)
                              (if (= (fn-arity (vector-ref cell 1)) 1)
                                  ((vector-ref cell 1) split)
                                  ((vector-ref cell 1) split transaction-row?))))

The code uses a guile-specific procedure called procedure-minimum-arity. It is not widely documented because not standard RnRS scheme but it was introduced in Guile version 2.0.0 (it is mentioned in the changelog for that version) and is till supported as of the current version 3.0.9. It’s pretty straightforward as explained by (help procedure-minimum-arity)
`procedure-minimum-arity' is a procedure in the (guile) module.

• Scheme Procedure: procedure-minimum-arity proc
  Return the "minimum arity" of a procedure.

  If the procedure has only one arity, that arity is returned as a
  list of three values: the number of required arguments, the number
  of optional arguments, and a boolean indicating whether or not the
  procedure takes rest arguments.

  For a case-lambda procedure, the arity returned is the one with the
  lowest minimum number of arguments, and the highest maximum number
  of arguments.

  If it was not possible to determine the arity of the procedure,
  `#f' is returned.

See the specific commit for this procedure: https://git.savannah.gnu.org/cgit/guile.git/commit/?id=cb2ce548441824fe1284fc80a3a95394a9fc03d0

For our purpose we can ignore optional and rest argument, and are only looking at the number of required argument, assuming either 1 (for legacy purpose) or otherwise expecting 2 as the else. Hence we simply retrieve the car of the procedure-minimum-arity for the provided calculator-functions to decide to either call it with 1 or 2 arguments. This therefore requires no changes to existing calculator-functions in trep-engine or trep-engine-based report but allows for addition of new calculator-functions with the 2 arguments.

I feel the addition of the transaction-row? argument is a worthwhile addition to the calculated-cells to bring them in line with the left-columns even outside of this current feature. For instance it could also be used to fix the running balance display in the multiline context or used by custom reports for other reasons.

Alternative to changes to add-row-split and calculator-functions’ number of arguments

If this change is considered to bold, the alternative would be to disable running totals in the multiline context for now, although that brings the smaller challenge of disabling “on the fly” the subtotal options in the ui (in the sorting section) when the “One Split per Line” is selected in the display section. I don’t think there is any way to do this right now while the dialog is active, or at least there is no hook I could find when changing tabs in the dialog (I could only get it done when it is opened the next time, or apply is clicked, or when a related option is clicked in the sorting window). That said the actual option can be disabled in the actual report output regardless of option graying in the dialog. So this is purely for the visual in the dialog and could possibly be left active with additional info in the option popup message stating it doesn’t work when “One Split per Line” is selected.

Changes to which number columns are formatted as links

The current implementation offers the “Enable Links” option in the display section. When enabled, it is applied to each cell that has a number in it. One limitation of this approach is that it is even applied to number cells whose calculated number is different from the transaction amount in the register (for instance a converted amount, or a running total). Also it can clutter the report visually since the link is essentially the same for each cell in a given row, pointing to the split in the register.

Ideally there would be an additional flag in the column definition to decide whether a link should or shouldn’t be present on that column, but that would require additional changes for such a small feature with potential impact on custom reports. The alternative I propose is to limit links (when enabled) to columns already marked as displaying a subtotal so as to declutter the report. To avoid an edge case of being left with no links on the report (which could happen if no columns with subtotals are enabled), the code reverts back to putting links on any number column in case no columns with subtotals are enabled.

[christopherlam]

Overall the code is fine in itself, however, it is not desirable to add too many options in such a complicated report. What about a single "Show separate primary/secondary/total subtotals?" option somewhere in the Sorting tab?

[dawansv]

Overall the code is fine in itself, however, it is not desirable to add too many options in such a complicated report. What about a single "Show separate primary/secondary/total subtotals?" option somewhere in the Sorting tab?

Yes the option dialog is getting crowded. That said keep in mind the total, subtotal options are already there. I am only adding 3 checkboxes, one each under their respective, already existing, totals or subtotal checkbox.

Now I can certainly look at reorganizing or combining some of these if that's OK (I was trying to reduce UI changes). For instance the Totals / runing total on the display could be a single drop down list instead of 2 checkboxes. The drop-down list would be called Totals and have 3 options: None, Totals, Running Total. Running totals automatically enables the totals line.

It could be similar for the subtotals except that then we have to deal with the "Primary/Secondary Subtotal to Date Key". So I would need to think more about how that would work.

[christopherlam]

Ok #1618 merged in this one just needs rebasing, refactoring and removal of arity code...

[dawansv]

Ok #1618 merged in this one just needs rebasing, refactoring and removal of arity code...

Great. I have rebased and updated my code. Tested and it all seems to work fine.

Also udated income-gst-statement.scm to disable runing total options in that context (that report uses all trep options by default, then removes the onces it doesn't one)

I still need to work on making some tests for the running totals

For your comment about the option dialog, I looked at simplifying the dialog but considering I am only adding 3 checkboxes (one under each of the 3 existing total/subtotal checkboxes) I am not sure I can simplify much. In particular the fact that the exsting subtotal options are already split between a checkbox and a drop-down list when the key is a date (to select the periods to subtotal: daily, weekly, monthly, etc) makes it difficult to combine the existing subtotal and new running subtotal options. So unless you have a more specific idea, then I don't see a way to improve this without redesigning the whole option panel which is not something I can embark on right now.

[dawansv]

FYI still need to rebase with your more recent changes to left-columns. Will do that later as it's bed time for me.

[dawansv]

I am trying to create a rather simple test from this html file (remove .log because I can't attached html here for some reason)
test-trep-running-total.html.log

The test simply grabs the last running total to make sure it equals the total of the test. It's -4 because there are 3 more rows for totals after that.

      ;; test running total
      (set! options (default-testing-options))
      (set-option! options "Display" "Running Total" #t)
      (let* ((sxml (options->sxml options "running total")))
        (test-equal "Last running total is $2,280.00"
          '("$2,280.00")
          (get-row-col sxml -4 -1)))
      )

Yet I keep getting this error, it returns #f instead of $2,880.00

[fail] line:708, test: Last running total is $2,280.00
transaction.scm/display options
-> expected: ("$2,280.00")
-> obtained: #f

Here is the kicker however, If I try row -5, the second to last line, I have no problem whatsover

      ;; test running total
      (set! options (default-testing-options))
      (set-option! options "Display" "Running Total" #t)
      (let* ((sxml (options->sxml options "running total")))
        (test-equal "Second-to-last running total is $2,302.00"
          '("$2,302.00")
          (get-row-col sxml -5 -1)))
      )

returns

[pass] line:708, test: Second-to-last running total is $2,302.00

I am puzzled to say the least...

[christopherlam]

The row/col analyses the xml tree exactly as written, rather than the rendered html table. This means a cell which spans 2 column will count as 1. To increase visibility of the xml try #f in row or col. (get-row-col sxml -5 #f).

Would it work better if the option "Table for export" is enabled?

[dawansv]

Summary

Here are a bunch of tests I ran with various results, but you can probably skip most of it and go to the last one at the bottom where I disable all totals, subtotals and runing total and yet I still get this odd behavior: every minus EVEN row fails and every minus ODD row works.

This is confirmed by my earlier tests where it just happened that the row I wanted was a minus EVEN, but when I removed one subtotal line and it moved to a minus ODD then suddently it works. Same thing happened with the columns, -2 (even) fails but -1 and -3 (odd) works.

Of note is that testing from TOP or LEFT (no minuses) works on all rows.

This is with table for exporting option on (makes no difference)

Notice (after the fact for me) how it's a MINUS EVEN that fails (-4)

      ;; test running total
      (set! options (default-testing-options))
      (set-option! options "Display" "Running Total" #t)
      (set-option! options "General" "Table for Exporting" #t)
      (let* ((sxml (options->sxml options "running total")))
        (test-equal "Second-to-last running total is $2,302.00"
          '("(get-row-col sxml -5 #f)")
          (get-row-col sxml -5 #f))
        (test-equal "Last running total is $2,280.00"
          '("(get-row-col sxml -4 #f)")
          (get-row-col sxml -4 #f))
        (test-equal "Bank total row"
          '("(get-row-col sxml -3 #f)")
          (get-row-col sxml -3 #f)))
      )

[fail] line:708, test: Second-to-last running total is $2,302.00
transaction.scm/display options
-> expected: ("(get-row-col sxml -5 #f)")
-> obtained: ("12/09/23" "$109 income" "Root.Asset.Bank" "$109.00" "$2,302.00")
[fail] line:711, test: Second-to-last running total is $2,280.00
transaction.scm/display options
-> expected: ("(get-row-col sxml -4 #f)")
-> obtained: #f
[fail] line:714, test: Bank total row
transaction.scm/display options
-> expected: ("(get-row-col sxml -3 #f)")
-> obtained: ("Total For Bank" "$2,280.00")

Testing from the top, it works

Exact same code but coming from the top (row 36 instead of -5, etc.) it works.

      ;; test from the top
      (set! options (default-testing-options))
      (set-option! options "Display" "Running Total" #t)
      (set-option! options "General" "Table for Exporting" #t)
      (let* ((sxml (options->sxml options "running total")))
        (test-equal "Second-to-last running total is $2,302.00"
          '("(get-row-col sxml 36 #f)")
          (get-row-col sxml 36 #f))
        (test-equal "Last running total is $2,280.00"
          '("(get-row-col sxml 37 #f)")
          (get-row-col sxml 37 #f))
        (test-equal "Bank total row"
          '("(get-row-col sxml 38 #f)")
          (get-row-col sxml 38 #f)))

[fail] line:724, test: Second-to-last running total is $2,302.00
transaction.scm/display options
-> expected: ("(get-row-col sxml 36 #f)")
-> obtained: ("12/09/23" "$109 income" "Root.Asset.Bank" "$109.00" "$2,302.00")
[fail] line:727, test: Last running total is $2,280.00
transaction.scm/display options
-> expected: ("(get-row-col sxml 37 #f)")
-> obtained: ("12/15/23" "$22 expense" "Root.Asset.Bank" "-$22.00" "$2,280.00")
[fail] line:730, test: Bank total row
transaction.scm/display options
-> expected: ("(get-row-col sxml 38 #f)")
-> obtained: ("Total For Bank" "$2,280.00")

Retrieving columns, second-to-last fails

(Here again MINUS EVEN fails)

      ;; testing columns
      (set! options (default-testing-options))
      (set-option! options "Display" "Running Total" #t)
      (set-option! options "General" "Table for Exporting" #t)
      (let* ((sxml (options->sxml options "running total")))
        (test-equal "Third column from the end"
          '("(get-row-col sxml #f -3)")
          (get-row-col sxml #f -3))
        (test-equal "Second-to-last column"
          '("(get-row-col sxml #f -2)")
          (get-row-col sxml #f -2))
        (test-equal "Last column"
          '("(get-row-col sxml #f -1)")
          (get-row-col sxml #f -1)))
      )

[fail] line:739, test: Third column from the end
transaction.scm/display options
-> expected: ("(get-row-col sxml #f -3)")
-> obtained: ("Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank")
[fail] line:742, test: Second-to-last column
transaction.scm/display options
-> expected: ("(get-row-col sxml #f -2)")
-> obtained: #f
[fail] line:745, test: Last column
transaction.scm/display options
-> expected: ("(get-row-col sxml #f -1)")
-> obtained: ("$103.00" "$212.00" "$190.00" "$293.00" "$402.00" "$380.00" "$483.00" "$592.00" "$570.00" "$673.00" "$782.00" "$760.00" "$863.00" "$972.00" "$950.00" "$1,053.00" "$1,162.00" "$1,140.00" "$1,243.00" "$1,352.00" "$1,330.00" "$1,433.00" "$1,542.00" "$1,520.00" "$1,623.00" "$1,732.00" "$1,710.00" "$1,813.00" "$1,922.00" "$1,900.00" "$2,003.00" "$2,112.00" "$2,090.00" "$2,193.00" "$2,302.00" "$2,280.00")

Retrieving columns from the left works

[fail] line:739, test: Third column from the end
transaction.scm/display options
-> expected: ("(get-row-col sxml #f 5)")
-> obtained: ("Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank" "Root.Asset.Bank")
[fail] line:742, test: Second-to-last column
transaction.scm/display options
-> expected: ("(get-row-col sxml #f 6)")
-> obtained: ("$103.00" "$109.00" "-$22.00" "$103.00" "$109.00" "-$22.00" "$103.00" "$109.00" "-$22.00" "$103.00" "$109.00" "-$22.00" "$103.00" "$109.00" "-$22.00" "$103.00" "$109.00" "-$22.00" "$103.00" "$109.00" "-$22.00" "$103.00" "$109.00" "-$22.00" "$103.00" "$109.00" "-$22.00" "$103.00" "$109.00" "-$22.00" "$103.00" "$109.00" "-$22.00" "$103.00" "$109.00" "-$22.00" "$2,280.00" "$2,280.00")
[fail] line:745, test: Last column
transaction.scm/display options
-> expected: ("(get-row-col sxml #f 7)")
-> obtained: ("$103.00" "$212.00" "$190.00" "$293.00" "$402.00" "$380.00" "$483.00" "$592.00" "$570.00" "$673.00" "$782.00" "$760.00" "$863.00" "$972.00" "$950.00" "$1,053.00" "$1,162.00" "$1,140.00" "$1,243.00" "$1,352.00" "$1,330.00" "$1,433.00" "$1,542.00" "$1,520.00" "$1,623.00" "$1,732.00" "$1,710.00" "$1,813.00" "$1,922.00" "$1,900.00" "$2,003.00" "$2,112.00" "$2,090.00" "$2,193.00" "$2,302.00" "$2,280.00")

Without subtotals

If I turn off the subtotals (so I have only 2 total rows at the bottom instead of 3), then it is STILL row -4 that fails (which is now is the second-to-last in the transaction list). My last row (previous row -4) is now -3 and it comes back just fine.

      ;; test running total
      (set! options (default-testing-options))
      (set-option! options "Display" "Running Total" #t)
      (set-option! options "General" "Table for Exporting" #t)
      (set-option! options "Sorting" "Primary Subtotal" #f)
      (set-option! options "Sorting" "Primary Subtotal for Date Key" 'none)
      (set-option! options "Sorting" "Secondary Key" 'none)
      (set-option! options "Sorting" "Secondary Subtotal" #f)      
      (let* ((sxml (options->sxml options "running total")))
        (test-equal "Second-to-last running total is $2,302.00"
          '("(get-row-col sxml -4 #f)")
          (get-row-col sxml -4 #f))
        (test-equal "Last running total is $2,280.00"
          '("(get-row-col sxml -3 #f)")
          (get-row-col sxml -3 #f))
        (test-equal "Bank total row"
          '("(get-row-col sxml -1 #f)")
          (get-row-col sxml -1 #f)))
      )

[fail] line:713, test: Second-to-last running total is $2,302.00
transaction.scm/display options
-> expected: ("(get-row-col sxml -4 #f)")
-> obtained: #f
[fail] line:716, test: Last running total is $2,280.00
transaction.scm/display options
-> expected: ("(get-row-col sxml -3 #f)")
-> obtained: ("12/15/23" "$22 expense" "Root.Asset.Bank" "-$22.00" "$2,280.00")
[fail] line:719, test: Bank total row
transaction.scm/display options
-> expected: ("(get-row-col sxml -1 #f)")
-> obtained: ("Grand Total" "$2,280.00")

Final test: With no subtotals or totals

OK so forger about totals and running totals. Here is a version with all of that disabled. So just a very basic table.
test-trep-running-total.html.log

Yet here we see that testing the last 6 rows, every -EVEN fails while -ODD works

      ;; test minus odd and even rows
      (set! options (default-testing-options))
      (set-option! options "Display" "Totals" #f)
      (set-option! options "General" "Table for Exporting" #t)
      (set-option! options "Sorting" "Primary Subtotal" #f)
      (set-option! options "Sorting" "Primary Subtotal for Date Key" 'none)
      (set-option! options "Sorting" "Secondary Key" 'none)
      (set-option! options "Sorting" "Secondary Subtotal" #f)      
      (let* ((sxml (options->sxml options "running total")))
        (test-equal "Row -6"
          '("(get-row-col sxml -6 #f)")
          (get-row-col sxml -6 #f))      
        (test-equal "Row -5"
          '("(get-row-col sxml -5 #f)")
          (get-row-col sxml -5 #f))
        (test-equal "Row -4"
          '("(get-row-col sxml -4 #f)")
          (get-row-col sxml -4 #f))
        (test-equal "Row -3"
          '("(get-row-col sxml -3 #f)")
          (get-row-col sxml -3 #f))
        (test-equal "Row -2"
          '("(get-row-col sxml -2 #f)")
          (get-row-col sxml -2 #f))          
        (test-equal "Row -1"
          '("(get-row-col sxml -1 #f)")
          (get-row-col sxml -1 #f)))
      )

[fail] line:713, test: Row -6
transaction.scm/display options
-> expected: ("(get-row-col sxml -6 #f)")
-> obtained: #f
[fail] line:716, test: Row -5
transaction.scm/display options
-> expected: ("(get-row-col sxml -5 #f)")
-> obtained: ("11/09/23" "$109 income" "Root.Asset.Bank" "$109.00")
[fail] line:719, test: Row -4
transaction.scm/display options
-> expected: ("(get-row-col sxml -4 #f)")
-> obtained: #f
[fail] line:722, test: Row -3
transaction.scm/display options
-> expected: ("(get-row-col sxml -3 #f)")
-> obtained: ("12/03/23" "$103 income" "Root.Asset.Bank" "$103.00")
[fail] line:725, test: Row -2
transaction.scm/display options
-> expected: ("(get-row-col sxml -2 #f)")
-> obtained: #f
[fail] line:728, test: Row -1
transaction.scm/display options
-> expected: ("(get-row-col sxml -1 #f)")
-> obtained: ("12/15/23" "$22 expense" "Root.Asset.Bank" "-$22.00")

[christopherlam]

Please feel free to add your WIP test commit. I may need to get back to this 2wks from now.

[dawansv]

Please feel free to add your WIP test commit. I may need to get back to this 2wks from now.

Will do and no rush. I can also most do the test without the minus row calls but will also keep investigating a bit. I don't find a lot of existing tests using a MINUS EVEN but I did find one, test-trial-balance.scm is calling "'sxml->table-row-col sxml 1 -2 #f" so I will have a look at that to see if I can find a clue there.

[dawansv]

Rebased to 5.1. Tests included. Moving this out of draft.

As for the strange issue with the "minus even" rows and columns in the test, I am not using these in my test anymore. I will submit a separate PR to investigate this further since I can reproduce it on a much simpler example.
Edit: See PR #1621

[christopherlam]

Could you possibly perhaps merge into 3 separate commits to tidy this branch for merging? (1) the triad of running balances with tests (2) trep to filter cell-with-subtotals? with its own test, and (3) income-gst-statement ?

[dawansv]

Could you possibly perhaps merge into 3 separate commits to tidy this branch for merging? (1) the triad of running balances with tests (2) trep to filter cell-with-subtotals? with its own test, and (3) income-gst-statement ?

Yes good idea.

[dawansv]

OK done with merge in 3 commits.

[christopherlam]

Without delving too much into the technicalities of the old vs new running total, I wonder if the number of options can be minimised by assuming that if the Display option specifies Running Total, then the visibility of Primary/Secondary/Total running totals depends on the existence of primary/secondary sort keys. IOW if secondary sort-key is None then disable its running total. Would this help?

[gjanssens]

@christopherlam asked me to voice my opinion as well 😉 as I'm known as the 'option guy' around here, typically raising concerns about adding options. I'll admit I have no strong opinion on this one as I'm not using these reports and haven't touched its code.

If I understand correctly your feature enables running balances of three levels: globally, and at each grouping level. Your intro explains what they do, but not why they are useful in this report. So I guess my first question is which use cases do you target with each of these three running sums ?

[dawansv]

If I understand correctly your feature enables running balances of three levels: globally, and at each grouping level.

Your understanding of the feature is correct except possibly for the running balance vs running total part. Strictly speaking, a running balance (at least of the context of accounting) is related to an account balance, hence much more specific; this is the way the running balance works in the report. It simply takes the account balance for each transaction. Hence why it needed to be "fixed" in terms of presentation because it is only running when the transaction report is filtering, grouping and sorting exactly the same transaction as the account registers. This is what we did already in PR #1604. Hence the running balance feature is actually quite specific to a single use case: when the transaction report matches an account register (or several if using subtotals). Otherwise it's just showing the account balance at each transaction

Running totals have nothing to do with account registers nor how the underlying data is sorted outside of the report. It starts with a list of numbers in whatever order that list is presented, in this case whatever filtering, grouping, ordering, sorting, the transaction report is giving us. I assume you already know the transaction report can literally filter, group, sort, and order transaction in almost any imaginable way without any connection to the underlying registers. I could filter all transactions with the word "blue in them in a specific selection of accounts, sorted by description ascending.

We shall define the resulting list of transactions as (1..n). Each time there is a list of items and a total (in any context), their can be a running total. Technically. taking the list (1..n), the total is the sum of (1..n) while the running total is the sum at every point down the list; (1), sum(1..2), sum(1..3), sum(1..4), etc until sum(1..n); Hence by definition the last running total of a series is always equal to the total of the series. There isn't much more to it than that.

So I guess my first question is which use cases do you target with each of these three running sums ?

First there are 3 levels of running totals because there are 3 levels of totals. (edit: It's worth adding that the subtotal levels typically show more than one total each, consecutively, since they are subtotals, and the same applies to running totals). I don't think users are likely to use all 3 at once, but since we give them a choice of any of these 3 levels of totals it seemed to make sense to give them a choice of any of the same 3 levels of running totals.

As to what the use case of any of these running totals is, I could say it's the same as the use case for the totals themselves, but extended.

For instance; I have a list of expenses (of a certain type I selected using all of the report feature I have; it doesn't have to be expenses for a given account). We had agreed only to spend $500 for all of these things I have selected.
What's the use case for the total? Well it tells me how much we actually spent on them. Imagine it's $750.
What's the use case for the running total? It's the same but more. It tells me how much we sent on them (the last running total is = to the total) but also where in the list we went over the $500.

             Amount  Running Total
Expense 1    $100    $100
Expense 2    $150    $250
Expense 3    $300    $550
Expense 4    $200    $750
TOTAL        $750

This is just one use case. I could come up with more but to be honest running total is just a generic concept, just as total is, I am hoping the case is made with just this.

[dawansv]

Without delving too much into the technicalities of the old vs new running total

Indeed but again there never was an old running total. There was and still is a a running account balance (different use case in a restricted filtering and sorting context).

The visibility of Primary/Secondary/Total running totals depends on the existence of primary/secondary sort keys

Worth noting that this is already the way in one direction: the running total for a given level (Primary/Secondary/Grand Total) is only available if the total for that level is enabled.

So I am not against that idea going the other way as well, meaning if you turn running totals on, you turn it on for all levels where totals are enabled. It's a bit more restrictive but if that's the compromise, I am fine with it. In this case yes, instead of one additional checkbox for each level we only have one checkbox which I assume would be the one on the display panel currently only targeting the grant total.

An alternative: Still only on the display and still a single option, but a drop down list with the following options:

Running Total...
* None
* Grand Total Only
* Grand Total and Subtotals

You still lose the granularity of one subtotal level vs the other but to be honest I am not sure this is that important. I can certainly show what that one would look like (we already know what the other one with just the check box would look like since it's the same as of now on the display tab)

[dawansv]

Also just to add, since we are discussing option presentations. The current (not mine) option to just turn on the Grand Total on the Display tab says "Totals". Note the plural form. I don't understand why it does say that. It only turns on the "Grand Total" which by definition there is only one of for each report. That option has no impact on the totals for the other 2 levels that are enabled on the sorting display. Personally I feel it would be best to rename that option "Grand Total" which is what it is called in the report itself.

Edit: OK on second thought the plural could refer to more than one column being totaled, hence totals as in several columns on the Grant Total line. Still, in this context, Grand Total (singular) still makes more sense. Yes several columns might have a grand total, but there is still ONE Grand Total line (hence the singular). Totals still just makes it sound as if it would control the subtotals as well which it doesn't. Anyway just my brain going in overdrive now so possibly just forget about this comment.

[jralls]

It should be consistent. If you make it Grand Total you don't have to change the subtotal labels on the Sorting page.

[dawansv]

I have combined all running totals options into one option at the bottom of the display tab. Also renamed Totals to Grand Total.

[christopherlam]

Nice. Now don't forget the options renamer mechanism somewhere in the options cpp code.

[dawansv]

Nice. Now don't forget the options renamer mechanism somewhere in the options cpp code.

The option renaming is the the last commit. Actually I have a question about this. Since the rename is not report specific, what would happen if 2 reports use 2 unrelated options with the same name; then one is renamed and the other is not; or worse both are renamed differently...

[jralls]

Each report gets its own optiondb instance so name collisions can't happen.

[dawansv]

Each report gets its own optiondb instance so name collisions can't happen.

OK but isn't there only one global renaming list in gnc-optiondb.cpp? Look at my last commit 6eaa2fe ; if there was also a completely different report with a completely different options called "Totals" that I wanted to rename to "Big Totals", how and where would I differentiate that with my "Totals" to "Grand Total" renaming as it is currently coded?

[jralls]

Ah, sorry I misunderstood. No, there's only one alias possible for any particular string.

There are three non-trep reports that have "Totals" options: General Journal, General Ledger, and Register. The General Journal's and Register Report Totals options don't seem to do anything, the General Ledger's option produces a new line named "Grand Totals". One might argue that you should apply your Grand Totals change to the General Ledger report and (in a separate commit) remove the Totals option from the General Journal and Register reports.

That doesn't solve the more general problem; I suppose that the right way to fix that would be to make the alias list mutable and add instances that shouldn't apply to all reports from the report that needs it. I don't think we should add that complication until we really need it.

@christopherlam?

[christopherlam]

Agree the current option renamer mechanism doesn't discriminate between different reports, and I don't particularly think this is currently an issue at this time. Hence I've typically preferred to use long report option names to be as descriptive as possible in any particular report.

[christopherlam]

I have no further comments on this branch 👍

[dawansv]

I squashed these last 2 commits into the main one. I think this is good to go then.

[christopherlam]

All good :)

[gjanssens]

I didn't get back to this PR in time. I'll just end with a 'nice work' compliment :)