2016-12-01 GnuCash IRC logs

00:11:10 *** O01eg has quit IRC
00:21:26 *** mlncn has quit IRC
00:43:28 *** BruiserTom has joined #gnucash
01:14:36 *** BruiserTom has quit IRC
01:35:46 *** mrklintscher has quit IRC
01:57:07 *** mrklintscher has joined #gnucash
02:06:38 *** gjanssens has joined #gnucash
02:06:38 *** ChanServ sets mode: +o gjanssens
03:36:24 *** warlord has quit IRC
04:28:07 *** fabior has joined #gnucash
04:50:52 *** fell has joined #gnucash
05:34:15 *** aqua has joined #gnucash
06:02:21 *** aqua has quit IRC
06:16:24 *** aqua has joined #gnucash
06:41:06 *** fabior has quit IRC
06:45:34 *** Jimraehl1 has joined #gnucash
06:46:16 *** aqua has quit IRC
06:56:06 *** fabior has joined #gnucash
07:00:29 *** aqua has joined #gnucash
07:02:17 *** mlncn has joined #gnucash
07:03:40 *** iliv has quit IRC
07:06:27 *** aqua has quit IRC
07:07:40 *** jchonig has quit IRC
07:17:47 *** iliv has joined #gnucash
07:20:35 *** aqua has joined #gnucash
07:28:59 *** jchonig has joined #gnucash
07:30:19 *** iliv has quit IRC
07:33:35 *** jchonig has quit IRC
07:42:01 *** aqua has quit IRC
07:44:17 *** fabior has quit IRC
07:51:11 *** warlord has joined #gnucash
07:51:11 *** gncbot sets mode: +o warlord
08:13:12 *** iliv has joined #gnucash
08:24:41 *** warlord has quit IRC
08:37:09 *** jchonig has joined #gnucash
08:43:17 *** fabior has joined #gnucash
09:03:53 *** fell has quit IRC
09:25:48 *** iliv has quit IRC
09:25:54 *** iliv has joined #gnucash
10:19:54 *** To7 has quit IRC
11:35:15 *** dgtlmoon has quit IRC
11:43:49 *** To7 has joined #gnucash
11:49:08 *** dgtlmoon has joined #gnucash
12:08:12 *** warlord has joined #gnucash
12:08:12 *** gncbot sets mode: +o warlord
12:16:43 *** mrklintscher has quit IRC
12:43:51 *** fabior has quit IRC
12:56:47 *** fabior has joined #gnucash
13:16:57 *** dgtlmoon has quit IRC
13:18:21 <jralls> gjanssens: IIUC you want to be able to set an arbitrary column as the one that the importer will use to tell whether adjacent rows are the same transaction or a new one, and you want the term to be a listbox member. What else goes in the listbox?
13:24:31 *** Mechtilde has joined #gnucash
13:33:21 *** dgtlmoon has joined #gnucash
13:43:43 *** kael has joined #gnucash
13:44:58 *** warlord has quit IRC
13:45:47 *** mlncn has quit IRC
13:49:11 *** aqua has joined #gnucash
13:53:46 <jralls> gjanssens/warlord/mikee: Could one or more of you take a look at https://bugzilla.gnome.org/show_bug.cgi?id=775368? I've managed to reproduce the problem and analyze it a bit, but my Scheme isn't good enough to find the actual problem.
13:56:55 *** fabior has quit IRC
14:08:36 *** aqua has quit IRC
14:09:48 *** dgtlmoon has quit IRC
14:09:50 *** kael has quit IRC
14:15:26 *** mlncn has joined #gnucash
14:21:26 *** aqua has joined #gnucash
14:25:28 *** dgtlmoon has joined #gnucash
14:41:47 <gjanssens> jralls: re my question - almost
14:42:47 <gjanssens> that arbitrary column won't be *the* column to indicate same/new transaction but *one of* the columns that together indicate same/new
14:43:01 <gjanssens> the other listbox members are here:
14:43:04 <gjanssens> https://github.com/gjanssens/gnucash/blob/cpp/src/import-export/csv-imp/gnc-trans-props.cpp#L56
14:43:42 <gjanssens> Note that in a more recent local commit I have changed "Other..." to "Transfer..." as those are the terms used in gnucash
14:45:08 <gjanssens> The long name may be a non-issue if I figure out how to tell the dropdown listbox from a combobox to be wider than the text entry field+arrow of that same combo box
14:46:01 <jralls> gjanssens: Hmm, but "TXN Dis.*" doesn't translate to a field so anything marked with it presumably wouldn't import. Is that your intent?
14:46:39 <gjanssens> yes. It's a dummy field
14:47:39 <gjanssens> It could for example point at a column with arbitrary numbers (which are consistently the same for all lines for one tx), or the gnucash guid, which I wouldn't import either
14:48:40 <gjanssens> It's only needed to solve the use case where a user would have two adjacent transacations that happen to have the same date and description
14:48:50 <jralls> But you would *export*, to be able to handle round-tripping txns with more than two splits, right?
14:49:00 <gjanssens> Like when you shop twice in the same shop on the same day
14:49:14 <gjanssens> Roundtripping is the other use case indeed. Yes, I would export
14:50:23 <jralls> I'd say that in the first use-case if the user has two txns in the the import then he wants to import two txns and we shouldn't be making him jump thorough hoops of creating a new column.
14:51:30 <gjanssens> Well, there's no other way I see gnucash can tell the difference at that point
14:51:54 <gjanssens> I'm trying to make the solution generic enough it works in many use cases
14:52:49 <gjanssens> But probably the roundtripping use case is the most likely one
14:53:14 <jralls> If there's no "Txn dis.*" column then every line is a separate transaction. If there is one then the importer is capable of > 2 splits per txn.
14:54:40 <gjanssens> If the source file had a transaction number column, the user wouldn't need a Txn Dis.* column, the num column would be the discerning column
14:55:08 <gjanssens> However in that case you can't identify it as Txn dis.*, because the user would want to import it as num instead
14:55:40 <gjanssens> The other more relevant use case would probably be importing from another accounting program btw
14:57:37 <gjanssens> So I went for a checkbox which the user can use to indicate whether or not the file is multi-split
14:57:59 <gjanssens> The available column types in the listbox would be adapted based on this choice.
14:58:23 <gjanssens> For the standard use case the Txn Dis.* option is not in the list
14:58:40 <gjanssens> In the multi-split scenario, the "Transfer *" options disappear
14:59:12 <gjanssens> As in that case each line defines exactly one split, so only one account is required
15:01:24 <jralls> But you still need to be able to get the values for the action and memo fields. Also better to provide "Debit" and "Credit" as well as "Deposit" and "Withdrawal". They can do the same thing.
15:02:00 <gjanssens> Action is currently not importable (but would be a simple addition)
15:02:32 <gjanssens> For memo theres "Memo" and "Transfer Memo", the second would be hidden in multi-split import
15:03:05 <jralls> I think it would be safe to use "Transaction ID" or "Transaction Identifier" as shorter versions of "Transaction Discriminator". The tooltip for the whole list can explain what to do with it.
15:03:07 <gjanssens> Debit & Credit are useful indeed. Something to add later.
15:05:02 <gjanssens> For importing multi-currency transactions (or stock related ones), we probably also need some form of amount vs value options and commodity selectors. But again such support will have to wait until later. It's not currently supported either.
15:06:52 <gjanssens> Ok, "Transaction ID" looks short enough. fell suggested that as well, but it may suggest one unique column, where I intend to support a multicolumn unique identifier (similar to a multi-column primary key in a db).
15:07:06 <gjanssens> I suppose if the tooltip explains this, that would be sufficient.
15:07:51 <gjanssens> This all brings me to the num vs action issue which got me scratching my head.
15:08:00 <jralls> What's the use-case for multi-column?
15:09:00 <gjanssens> Well, for example importing from a db in case it's hard to get a single column unique id
15:09:12 <jralls> If there's no capability to import action then the book's property applies: Num goes whereever that property says it should.
15:09:18 <gjanssens> It's a far fetched one, but easy to support
15:09:47 <gjanssens> Re num/action - indeed.
15:10:18 <jralls> And pointless. The user can just concatenate their multi-cloumn primary key into an id column. They'd need to do that anyway if any of the key columns need to be imported.
15:10:24 <gjanssens> However if the book setting says num goes to action->then it become a split property and no longer a transaction property
15:10:38 <jralls> Right. So?
15:11:11 <gjanssens> So I had nicely defined two structs - one for trans props, one for split props
15:11:24 <gjanssens> This num/action thing breaks the encapsulation once more
15:12:17 <gjanssens> And in the num is really action, the num field should probably not be used as a unique identier for multi-split transactions
15:12:32 <gjanssens> which means I have to implement all kinds of extra checks and exceptions
15:12:41 <gjanssens> which is ugly :(
15:13:22 <gjanssens> Still have to solve that part of the code by the way
15:14:31 <gjanssens> So, agreed. I will forget about multi column trans-id's. Makes it slightly easier to maintain.
15:14:47 <gjanssens> In that case Transaction ID will do fine.
15:14:56 *** fabior has joined #gnucash
15:15:05 <gjanssens> jralls: re https://bugzilla.gnome.org/show_bug.cgi?id=775368
15:15:19 <gjanssens> I can probably decipher what it does (given enough time)
15:15:50 <gjanssens> However I have no idea about what it *should* do
15:16:07 <gjanssens> I don't understand the average cost thing in itself
15:16:27 <gjanssens> So it'd be hard for me to spot what goes wrong...
15:18:41 <jralls> I think cstim wanted to collect all of the transactions in the book which involve two commodities and determine the average price/exchange-rate for each pair, weighted by the quantity of each transaction.
15:21:19 <jralls> That particular function should be adding up the amounts and values, but I think that maybe it's mishandling direction so that in the bug's example it finds an amount of 1/100 and a value of 1000/100. and so computes a rate of 1000/1.
15:22:16 <gjanssens> It should be getting 21000/100 and 19900/100 instead ?
15:23:09 <gjanssens> (well 19900/10 and 21000/100 to keep the order you started in)
15:23:22 <jralls> Maybe the ((caadr pair) 'add (cadr foreignlist)) or ((caadr pair) 'add value-amount) statements... I don't understand what those actually do.
15:23:58 <jralls> I think so.
15:26:45 <gjanssens> Hmm, the function looked familiar... I fixed another bug in it last year :)
15:27:05 <gjanssens> But I didn't dig through it completely then...
15:27:27 <gjanssens> When did this bug start ? Perhaps I introduced it :(
15:30:19 <jralls> Who knows? It was reported only a few days ago, but users have been complaining about "average cost" for as long as I've been on the project. I'm tempted to just rip it out as misguided.
15:31:59 <jralls> The implementation seems to be what the Brits call "too clever by half", and averaging the cost over the whole book isn't a normal thing to do.
15:35:42 <jralls> What's really needed is "book value", where the report/chart shows the acquisition cost of the asset rather than a price from the pricedb.
15:36:45 *** Mechtilde has quit IRC
15:37:45 <jralls> Mike A probably has the appropriate function in the Advanced Portfolio Report, we just need to move it over to report-system.
15:48:17 <jralls> gjanssens: I don't think you added the bug, you just added a line which excludes trading account splits.
15:55:00 *** mlncn has quit IRC
15:58:30 <gjanssens> jralls: I know. That fixed a bug where it was totally screwed with trading balances. Because trading splits always cancelled out any other foreign currency splits.
15:59:15 *** iliv has quit IRC
15:59:34 <jralls> So I've figured out that the lines that I didn't understand are calling the lambda buried in make-numeric-collector.
16:00:20 <gjanssens> Ok
16:00:49 <gjanssens> By the way the behaviour you're expecting is what is provided by the function right before in the same file
16:01:15 <gjanssens> gnc:get-exchange-totals does the calculation as we described above
16:01:32 <gjanssens> I don't know what average cost is supposed to do instead...
16:02:21 <jralls> Well, let's see if we can figure out what get-exchange-cost-totals is doing different.
16:02:52 <jralls> Aside from not ignoring trading accounts, that is.
16:03:57 <gjanssens> gnc-numeric-abs in share-amount and value-amount ?
16:04:53 <gjanssens> I believe the trading account exception should have been added to both functions
16:05:08 <gjanssens> But it probably wouldn't matter in gnc:get-exchange-totals
16:05:29 <gjanssens> Including the trading account splits would just double both amounts, resulting in the same average
16:05:31 <jralls> Yeah, that's one. Next is that it looks only at stock-type accounts. There's a note that that exception was removed from the cost function in v1.7.
16:09:42 <jralls> Otherwise the only difference is that the "else" clause of (if (not comm-list) is explicitly labelled. That's just sugar, right?
16:11:25 <gjanssens> No, that's because the in one case it's not (if (not comm-list) but (cond (not comm-list)
16:11:37 <gjanssens> (cond is like switch in c
16:11:46 <gjanssens> and (else is like default in that case
16:12:41 <gjanssens> The more important difference is in how the share-amount and value amount are added in foreign-list if the commodity already is in comm-list
16:14:10 <jralls> Ah, the gnc-numeric-neg?
16:14:20 <gjanssens> Yes
16:14:27 <gjanssens> That's only done in one case and not the other
16:14:52 <gjanssens> That is, if the account commodity is already in the list, the transaction commodity is added with the amounts negates
16:14:58 <gjanssens> negated*
16:15:11 <gjanssens> I don't know what the reasoning is behind that
16:15:47 <gjanssens> I understand the amounts are swapped depending on which commodity to add to the list, but I don't understand why it should be negated in one case
16:15:49 <jralls> Well, let's take it out and see what happens.... Aha! the right answer!
16:18:26 <jralls> Well, the right answer in the non-trading account case. Doesn't fix the trading account case. That's interesting.
16:20:12 <gjanssens> Hmm, perhaps my fix was the wrong one ?
16:20:26 <jralls> And that's because my no-trading-accounts case had a currency swap and the trading-accounts example from the bug report doesn't.
16:21:05 *** iliv has joined #gnucash
16:21:50 <jralls> Maybe. The "Weighted Average" option, which doesn't have your trading account fix, *does* give the right answer.
16:24:47 <jralls> But that also means that the "shares" check in that function is a no-op, because the example is pure currency. I think that's actually the case: it's based on xaccSplitGetAmount being 0/1, which only the case for capital gains splits. We probably want that in both functions.
16:27:17 <gjanssens> Ok
16:29:36 <jralls> Ah, the idea behind the negating would be because a reversed transaction is presumed to also reverse credit and debit, and credit is reflected as a neg number. So it just happened that for my particular test-case removing the negation happened to turn the cost function into the not-cost function.
16:30:04 <gjanssens> Although I don't follow what you mean by the "shares" check in that function being a no-op
16:31:52 <jralls> Sorry, too much thinking out loud. It isn't a no-op, it just doesn't do what I thought it did. It actually does the right thing and should be in both places.
16:32:18 <gjanssens> Ok, I figured that's was what you eventually concluded.
16:36:25 <jralls> So the difference between the (working) non-cost function and the (broken) cost function is that the non-cost one uses the absolute value of all of the transactions while the cost one cancels out buys with sells so computes only the net result, and that's whacky. I doubt that's what cstim thought he was doing. What I suspect he meant was to take only "buy" transactions into account. That would mean ignoring negative values
16:36:25 <jralls> after negating flipped splits.
16:36:44 <jralls> Did that make sense?
16:38:15 *** iliv has quit IRC
16:38:48 *** aqua has quit IRC
16:43:24 <gjanssens> If "cost" here should mean "buy" only, then yes, that's what should happen
16:43:50 <gjanssens> Does this still hold if the user is not consistent in his starting account ?
16:44:09 <gjanssens> Speaking in currency terms here, not shares
16:44:24 <gjanssens> If I buy USD for EUR and book this into my EUR account
16:44:50 <gjanssens> And then later sell USD for EUR again, but pay not attention and book that into my USD account instead
16:45:23 <gjanssens> So the transaction commodity is different in the two transactions
16:46:02 <gjanssens> I didn't think this through deeply, but perhaps that was something cstim was trying to accommodate ?
16:47:24 <jralls> gjanssens: Yes, that's what he was trying to do with the negation in the swap case. That's fine, the thing is that it needs to be checked for > 0/1.
16:49:26 <gjanssens> Ah, indeed
16:49:48 *** mlncn has joined #gnucash
16:51:49 *** aqua has joined #gnucash
16:52:04 <jralls> Or more correctly, the sign of the amount needs to be used to determine which comm-list the transaction goes into. A negative split should be inverted and swapped because it represents a buy of the other commodity.
16:55:02 <jralls> Rather than a reversed split because the transaction currency is in the "other" one should be negated and swapped because it's going the other way from the first one we encountered.
17:09:46 *** aqua has quit IRC
17:25:15 *** fabior has quit IRC
17:29:22 *** gjanssens has quit IRC
17:40:14 *** fabior has joined #gnucash
17:41:50 *** gnomey has quit IRC
17:45:55 *** warlord has joined #gnucash
17:45:55 *** gncbot sets mode: +o warlord
18:28:29 *** fabior has quit IRC
19:18:11 *** warlord has quit IRC
20:41:40 *** User_ has joined #gnucash
20:47:28 *** User_ has quit IRC
22:10:24 *** O01eg has joined #gnucash
22:23:31 *** O01eg has quit IRC
22:37:32 *** O01eg has joined #gnucash
23:13:24 *** mlncn has quit IRC
23:36:32 *** O01eg has quit IRC