728910: liabilities were adding as negative numbers

[datajunkie-eng]

For this ticket.

Here is the gnucash file that was attached to the ticket: test_data_from_ticket_get_rid_of_extension.gnucash.txt

Screenshots of the Budget Balance Sheet before and after the change:

[christopherlam]

Looks good. Care to amend the budget test suite?

[datajunkie-eng]

@christopherlam I've got this failure reproduced locally (sorry I'd not seen it before I submitted!) I'm trying to set up a file that has the same transactions/budget as the budget-bs unit test, and I've had some trouble with getting it to match even when I run the stable branch without this change. My best guess at this point is that it's using a different exchange rate for GBP-USD than the two rates in the test data, but I'm not sure. Digging through the code to figure it out has been pretty educational, though, so I'll just keep working it.

[christopherlam]

Thanks. Bug 728910 also requests some clarification on the headings, it would be useful try derive exactly their definition.

[datajunkie-eng]

Hi @christopherlam , I've made headway on the unit test and have it working, but I'm doubting that there should ever be negative income in a budget. I'd like to make them positive, but I wonder if I'm missing something.... maybe these are negative for a reason? There are a couple other tests in test-budget.scm that break when I change these to positive, and I'd be happy to fix them (and put together some additional documentation on the P&L budget report).

[christopherlam]

Hi the budget income values must typically be negative. After all negative income + positive assets into bank = a balanced budget equation.

[christopherlam]

There are a couple other tests in test-budget.scm that break when I change these to positive, and I'd be happy to fix them (and put together some additional documentation on the P&L budget report).

Fwiw these budget tests were written by myself a couple years back to "formalize" the current state of the buggy budget editor and reports, and were not intended to define the "authentic" intent of the budget reports. I still do not quite understand all the fields in the budget reports, hence I had not attempted to bug fix them.

As contributors such as yourself offer sensible clarifications and suggestions on fixing reports, I'd expect the tests to be suitably fixed too.

[datajunkie-eng]

Makes sense. I took a stab at documenting them here.

[datajunkie-eng]

Also, how the unit test knows the exchange rate for GBP->USD is still a mystery to me! I ended up manually entering a price to get the same numbers when running the report manually, but it'd be nice to know if that's actually defined in the test code, somewhere.

[christopherlam]

I've just tested and unfortunately this branch doesn't fix it correctly yet. It's sensitive to the Edit Preferences > Accounts > Reversed Balance Accounts setting, and shouldn't be.

[datajunkie-eng]

@christopherlam I'll have a look at the Reversed Balance issue.

[christopherlam]

If this is too difficult please feel free to create a spreadsheet with relevant formulas and attch to the bug.

[datajunkie-eng]

Hi @christopherlam , I've been reading up on the Reverse Balance setting, and have been comparing this report's behavior to the existing Balance Sheet. I don't think this is going to be too difficult but looking at the Balance Sheet I'm not clear what is the right path to take.

Here's a trivial example, the Standard Balance Sheet with the Reverse Accounts set to 'None':

It seems like Total Liabilities should match Liabilities (with both being negative), then the math for the Retained Earnings at the end giving a $12 for both cases. If that's true, what should I aim for in the Budgeted Balance Sheet: parity with the Balance Sheet, or correctness?

[christopherlam]

AFAIU the main balance-sheet.scm has been extensively tested and validated by multiple accounting professionals, so, I would aim to achieve parity with its figures.

[jralls]

I'm not sure I agree with that in this case. It doesn't look right that the liabilities account is unreversed but the liabilities total is reversed.

If you were to run the close-book tool on that book so that Retained Earnings is an Equity account instead of a calculated line it will also display as a negative number and there won't be a calculated Retained Earnings line. That's inconsistent at best. I think it would be better if the reports followed the sign-reverse preference for calculated amounts as well as for account balances.

[datajunkie-eng]

I've pushed some changes that defer all sign-sensitivity in until when the numbers are actually presented in the report, which makes it a lot easier to have computed fields that rely on other computed fields. I personally like this approach for sign-sensitive Liabilities fields better than how the Balance Sheet does it, but with the internal math being consistent now, it's easier to change it if that's the way people want to go. There was some other open discussion in the ticket that I'll work to move forward (possibly leading to more changes in this PR, I'm not sure how you all want to chunk it out) but thought this might at least be a good checkpoint.

[datajunkie-eng]

(rebased from stable branch)

[jralls]

I personally like this approach for sign-sensitive Liabilities fields better than how the Balance Sheet does it

So do I.

Perhaps the title of your first commit "liabilities were adding as negative numbers" needs changing: Of course they do. They are negative numbers.

with the internal math being consistent now

Maybe the internal math should be extracted to report-core.scm or report-utilities.scm and applied to all reports. @christopherlam ?

[datajunkie-eng]

@jralls you're right on the misnomer. I squashed the commits into one that's more appropriately titled.

[christopherlam]

I personally like this approach for sign-sensitive Liabilities fields better than how the Balance Sheet does it

So do I.

Perhaps the title of your first commit "liabilities were adding as negative numbers" needs changing: Of course they do. They are negative numbers.

Afaiu the balance-sheet and other legacy reports use html-acct-table as a general tool to walk the hierarchy of accounts and add onto the html table. I have never decoded how this handles sign negation especially when e.g. liability accounts have asset accounts as children.

with the internal math being consistent now

Maybe the internal math should be extracted to report-core.scm or report-utilities.scm and applied to all reports. @christopherlam ?

Maybe... but since this particular computation is only used in the budget balance sheet I see no particular need to extract the sums. What this file needs is much better documentation!

[jralls]

Thanks!