Exact assertions & multi commodity balances #871
Conversation
If multiple amounts of the same commodity are included in a single amount, they should be condensed to their sum; this has the side benefit of allowing arithmetic in postings.
As commas are potential decimal separators, using them as amount
separators throws off the parsing with the current precedence
("10.25, 4" is read as "1025" with a precision of 0, before getting
confused at the "4") unless we force the slightly-unnatural syntax of
space-comma. It is likely possible to work around that, but the parsing
that would be required is particularly complex, and the simplicity
doesn't outweigh the risk of bugs.
'+' was chosen over '/' to both reflect the behaviour when one commodity
is listed multiple times, as arithmetic is a more accurate conception
than alternatives, and to potentially allow '-' in the future to extend
the allowed math.
"brick" constraint is only arbitrary; should be revisited when someone has time to focus on infrastructure.
|
Ah, didn't notice that test suite. Not quite sure what happened to the rewrites, but I'll track that down next. |
ag-eitilt
force-pushed
the
multi-commodity
branch
from
18ae9e2
to
d7f9028
Compare
simonmichael
added
A-WISH
|
Thanks for the PR! Tip: use This looks like a WIP branch, which might get cleaned up before merge, so I'll review with that in mind. For reviewers, can you summarise the effect of this PR, eg: "it allows posting amounts and balance assertion amounts to be written as arithmetic expressions" ? |
|
I think I'm right in saying the impact of these commits is: Adds amount expressions:
Adds exact multi-commodity assertions:
|
Doesn't actually do anything yet, but is less heavy-handed than the other method I was trying.
| `$6.10 + £4.35 * 0.8 €` -> `$6.10 + 3.48 €` | ||
| `($6.10 + £4.35) * 0.8 €` -> `8.36 €` | ||
| `$6.10 * (£0.2 + 0.8 €)` -> `£1.22 + 4.88 €` | ||
|
|
There was a problem hiding this comment.
I think folks will immediately want division (/) as well, can you add that ? We'll need to handle division by zero in some reasonable manner.
There was a problem hiding this comment.
Adding it won't be hard at all, except for divide-by-zero; the operation parser wound up being amazingly extensible. I'm tempted to just make it silently fail, but it's probably better to work through the error reporting system to ensure sanity. I've not looked at that yet, though, so I might put division off until the end of the refactor, if you think that's all right.
There was a problem hiding this comment.
division can be a problem too... What would be the amount for $0.05 / 3 ?
There was a problem hiding this comment.
I meant: 0.016666666666666666...
| applied to the posting value). | ||
|
|
||
| Any postings in real transactions which attempt to use this | ||
| multiplication-headed form will ignore the value instead. |
There was a problem hiding this comment.
I didn't understand "headed by" onward exactly. Shall we simplify again, deleting those lines ? Ie we will accept an amount, an amount expression, or a numeric multiplier (*N). (There'll probably be desire for \N, +N, -N, arbitrary arithmetic functions, referencing other parts of the transaction etc., but those can wait)
There was a problem hiding this comment.
Blame a couple years of linguistics. That's the passage that was primary in the "first draft" naming of the commit. I'll revisit this.
There was a problem hiding this comment.
On the "those can wait", I might actually put the first step of the PR together without multiplication entirely. Addition and subtraction are a much more contained change, and seem to generate much less discussion (while being more frequently requested). We should definitely keep working out the questions around multiplication, but splitting things further like that means there's less pressure to resolve it so we can find something we're all happier with.
simonmichael
added
needs:discussion
|
So, this week is going to be mostly focused on smaller refactoring as well, starting with splitting out the assertation portion. I'll leave this open for now to not lose the conversation, but once the amount expressions get cleaned up, I'll close this PR; that seems the best way to cut through the tangle. [new PR: #902] |
|
On Fri, Oct 12, 2018 at 12:12:04PM +0000, mildred wrote:
mildred commented on this pull request.
...
+As examples, with the value each expression simplifies to (if
+applicable):
+
+`$47.18-$7.13` -> `$40.05`
+`$47.18 - ($20 + $7.13)` -> `$20.05`
+`$6.10 + £4.35 * 0.8 €` -> `$6.10 + 3.48 €`
+`($6.10 + £4.35) * 0.8 €` -> `8.36 €`
+`$6.10 * (£0.2 + 0.8 €)` -> `£1.22 + 4.88 €`
+
division can be a problem too... What would be the amount for `$0.05 / 3` ?
this should fail, unless you explicitly specified the rounding to be
applied! e.g. at home in our meta-ledger input we use ++ and -- at
the end of such expressions to indicate ROUND_UP or ROUND_DOWN.
'$0.05 / 3' >> runtime error
'$0.05 / 3 --' >> $0.01
'$0.05 / 3 ++' >> $0.02
'€1 / 3' >> runtime error
'€1 / 3 --' >> €0.33
'€1 / 3 ++' >> €0.34
ofcourse ++ and -- are only allowed once at the end of an expression
There is no rounding towards zero or fancy things like fair rounding,
only plain UP and DOWN. At home we always have two decimal digits but
maybe the number of decimal digits should be commodity specific.
fair rounding are schemes to compensate for rounding ups preference to
round up, i.e halfway (.5) always rounds up. Bankers might like them,
but they complicate things, and explicit seems better then implicit.
…--
groetjes, carel
|
|
We don't currently support rounding strategies, that would be a separate discussion. We store arbitrary decimal numbers (up to 255 decimal places IIRC), and we display them with the number of decimal places inferred or specified for each commodity. |
simonmichael
removed
needs:testing
simonmichael
added
needs:discussion
simonmichael
added
needs:external-task
simonmichael
removed
needs:external-task
I'm not entirely sure how "lay the groundwork for #556" became "implement #183 and #861", especially since doing so was only a side effect of putting multiple commodities on a single line. That, at least, can be traced to what @simonmichael mentioned about halfway down #195, but that still begs the question of why I implemented exclusive balance assertions before what I actually set out to fix. Either way, it seems to work, unless I missed testing some strange interaction somewhere. And with minimal journalfile breakage, as well: commodities including parentheses need to be quoted now, but that might actually bring things more in line with ledger, depending on what its arithmetic expressions disallow.