[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 Ralf Habacker changed: What|Removed |Added Version Fixed In|5.0.0 |4.8.0 -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 Ralf Habacker changed: What|Removed |Added Version Fixed In||5.0.0 CC||ralf.habac...@freenet.de -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 NSLW changed: What|Removed |Added Latest Commit|http://commits.kde.org/kmym |http://commits.kde.org/kmym |oney/d9a042a0322a7d76633f0c |oney/d6ac96c17fe5d4af18e34b |9bc53a367036ed684a |f63635a810cafb4ca0 |http://commits.kde.org/kmym |http://commits.kde.org/kmym |oney/d6ac96c17fe5d4af18e34b |oney/6605675815914296c5ebc9 |f63635a810cafb4ca0 |b7d8e90658de1708a2 --- Comment #18 from NSLW --- Git commit d6ac96c17fe5d4af18e34bf63635a810cafb4ca0 by Łukasz Wojniłowicz. Committed on 28/05/2016 at 18:27. Pushed by wojnilowicz into branch 'frameworks'. Allow importing negative sell transaction Buy transactions should be negative and sell transactions are generally positive but can be negative if fees are higher than proceeds. Let OFX, QIF, and CSV importer and not statement reader decide about transaction amount signs. REVIEW: 127983 Signed-off-by: Łukasz Wojniłowicz M +9-17 kmymoney/converter/mymoneystatementreader.cpp http://commits.kde.org/kmymoney/d6ac96c17fe5d4af18e34bf63635a810cafb4ca0 --- Comment #19 from NSLW --- Git commit 6605675815914296c5ebc9b7d8e90658de1708a2 by Łukasz Wojniłowicz, on behalf of Jeff Lundblad. Committed on 28/05/2016 at 18:26. Pushed by wojnilowicz into branch 'frameworks'. Set signs properly for buy/sell transactions in QIF importer Send to statement reader negative buy transactions and positive or negative sell transactions, which is dependend on fees and proceeds relationship. REVIEW: 124957 Signed-off-by: Łukasz Wojniłowicz M +4-1kmymoney/converter/mymoneyqifreader.cpp http://commits.kde.org/kmymoney/6605675815914296c5ebc9b7d8e90658de1708a2 -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 NSLW changed: What|Removed |Added Latest Commit|http://commits.kde.org/kmym |http://commits.kde.org/kmym |oney/d9a042a0322a7d76633f0c |oney/d6ac96c17fe5d4af18e34b |9bc53a367036ed684a |f63635a810cafb4ca0 |http://commits.kde.org/kmym |http://commits.kde.org/kmym |oney/d6ac96c17fe5d4af18e34b |oney/6605675815914296c5ebc9 |f63635a810cafb4ca0 |b7d8e90658de1708a2 --- Comment #18 from NSLW --- Git commit d6ac96c17fe5d4af18e34bf63635a810cafb4ca0 by Łukasz Wojniłowicz. Committed on 28/05/2016 at 18:27. Pushed by wojnilowicz into branch 'frameworks'. Allow importing negative sell transaction Buy transactions should be negative and sell transactions are generally positive but can be negative if fees are higher than proceeds. Let OFX, QIF, and CSV importer and not statement reader decide about transaction amount signs. REVIEW: 127983 Signed-off-by: Łukasz Wojniłowicz M +9-17 kmymoney/converter/mymoneystatementreader.cpp http://commits.kde.org/kmymoney/d6ac96c17fe5d4af18e34bf63635a810cafb4ca0 --- Comment #19 from NSLW --- Git commit 6605675815914296c5ebc9b7d8e90658de1708a2 by Łukasz Wojniłowicz, on behalf of Jeff Lundblad. Committed on 28/05/2016 at 18:26. Pushed by wojnilowicz into branch 'frameworks'. Set signs properly for buy/sell transactions in QIF importer Send to statement reader negative buy transactions and positive or negative sell transactions, which is dependend on fees and proceeds relationship. REVIEW: 124957 Signed-off-by: Łukasz Wojniłowicz M +4-1kmymoney/converter/mymoneyqifreader.cpp http://commits.kde.org/kmymoney/6605675815914296c5ebc9b7d8e90658de1708a2 -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 NSLW changed: What|Removed |Added Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED Latest Commit|http://commits.kde.org/kmym |http://commits.kde.org/kmym |oney/8b3eac05eaa8437215232d |oney/8b3eac05eaa8437215232d |fd4f7ddb3da5c49ad8 |fd4f7ddb3da5c49ad8 ||http://commits.kde.org/kmym ||oney/d9a042a0322a7d76633f0c ||9bc53a367036ed684a --- Comment #16 from NSLW --- Git commit 8b3eac05eaa8437215232dfd4f7ddb3da5c49ad8 by Łukasz Wojniłowicz, on behalf of Jeff Lundblad. Committed on 28/05/2016 at 17:48. Pushed by wojnilowicz into branch 'master'. Set signs properly for buy/sell transactions in QIF importer Send to statement reader negative buy transactions and positive or negative sell transactions, which is dependend on fees and proceeds relationship. REVIEW: 124957 Signed-off-by: Łukasz Wojniłowicz M +4-1kmymoney/converter/mymoneyqifreader.cpp http://commits.kde.org/kmymoney/8b3eac05eaa8437215232dfd4f7ddb3da5c49ad8 --- Comment #17 from NSLW --- Git commit d9a042a0322a7d76633f0c9bc53a367036ed684a by Łukasz Wojniłowicz. Committed on 28/05/2016 at 17:53. Pushed by wojnilowicz into branch 'master'. Allow importing negative sell transaction Buy transactions should be negative and sell transactions are generally positive but can be negative if fees are higher than proceeds. Let OFX, QIF, and CSV importer and not statement reader decide about transaction amount signs. REVIEW: 127983 Signed-off-by: Łukasz Wojniłowicz M +9-17 kmymoney/converter/mymoneystatementreader.cpp http://commits.kde.org/kmymoney/d9a042a0322a7d76633f0c9bc53a367036ed684a -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #15 from Jeff --- With my testing, the combination described in comment #14 seems to work. -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #14 from NSLW --- Jeff please test your patch from review #124957 together with my patch from review #127983 atop git master branch to see if you can import your QIF and OFX files properly. I hope we will be able to close this bug soon. -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #13 from Jeff --- > With your suggested change, to remove the sign reversal, when the > transaction imports, the result shows as a deposit of $8.77 (positive). In order for that 8.77 to properly become a payment instead of a deposit, I have to restore the statement reader to its 6/28/2015 version as well as remove the negative sign on the line in the QIF reader (and to make it all work, place a negative sign on the "buy" case in the QIF importer). It then imports with a warning triangle, but simply hitting "Edit" followed by the "Enter" fixes the math and the warning goes away. To remove the warning and have the whole thing work would probably take some different change to the statement reader that I have not pursued. (I'm guessing the 6/28/2015 statement reader has some extraneous .abs() when it sets up the split and the transaction editor does not, which is why the editor fixes it.) As I recall from my quick look, the current master branch version of the statement reader makes assumptions about buys and sells and basically ignores the signs on the incoming data. So you could change all the signs in the QIF importer and the ledger results would not change. -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #12 from allan --- (In reply to Jeff from comment #9) > That line: > > tr.m_amount = -amount; > > is the one I changed in my patch to fix my problem for "sells". Need remove > the negative sign. Sorry, but I'm a bit puzzled here. With your suggested change, to remove the sign reversal, when the transaction imports, the result shows as a deposit of $8.77 (positive). Throughout the import, the negative sign is maintained, as far as I can see. So, I don't see "The cash account should decrease by 8.77." Why don't I see the result you are expecting? -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #11 from Jeff --- > I wonder how you test if OFX imports are correct? Is your test based only on > fact that KMM doesn't show you warning signs in ledger? This wasn't addressed to me, but I will chime in. I know OFX imports work because I have 2 investment accounts that I trade in quite frequently (daily to weekly), 2 more that I trade in fairly often (monthly), and 4 more that get updates every few months. I know the imports are working because my stock and cash balances in KMM match the balances at the brokers. > Summarizing it: It didn't work for you in general case and corner case. Now > it works for you in general case but still not in corner case :) I believe that is correct. But I didn't do extensive testing before I backed your change out of my copy. -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #10 from NSLW --- (In reply to Jack from comment #5) > I import to investment accounts from OFX frequently, and it works just fine. > (Well, mostly, but my problems are mainly in what my broker provides, not > how KMM handles it, and I've complained about it in the past on the mailing > list.) I can probably provide some example files, but I'd have to choose > carefully for ones that don't show any of the problems. I wonder how you test if OFX imports are correct? Is your test based only on fact that KMM doesn't show you warning signs in ledger? My problem with CSV was that buy and sell amounts with commissions were wrong but warning sing was shown only for sell transactions. I patched CSV code in such way that no warning sign was shown, but both buy and sell amounts still were wrong. In my opinion, I tried every combination in CSV code to make amounts right but I failed, so statement reader was to blame. I'm going to code on weekend, so your OFX file would help me embrace it all at one time. (In reply to Jeff from comment #6) > There is still a problem with the QIF import with your change. My test file > also tested the case where the commission was greater than the proceeds from > the sale (which can happen when trading options.) Your fix changed a "sell" > trade that actually cost money into one that brought in money. The example > in my test file was the "sell" of the "NFLX Aug 18 2012 110.0 Call". The > price is 0.02, times 100 shares = 2.00. The commission is 10.77. So income > of 2.00, outgo of 10.77 makes the total -8.77 (as shown in the U and T > values in the QIF file). The cash account should decrease by 8.77. Your > change turned that into a positive 8.77, and increased the cash account. > This is admittedly a corner case, and I think I am the only KMM user that > trades options because I have made a bunch of other changes to the KMM code > to support that. Now I see that too and it needs to be fixed. As you've said it's corner case and I didn't take it into account. Summarizing it: It didn't work for you in general case and corner case. Now it works for you in general case but still not in corner case :) (In reply to allan from comment #7) > The OFX specification 2.0.3 includes - > "CHAPTER 13 INVESTMENTS > OFX supports download of security information and detailed investment > account statements including > transactions, open orders, balances, and positions. > " plus a lot more in detail. > Allan Thanks for the info. It looks like I was searching in the wrong area. Regards Łukasz -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #9 from Jeff --- That line: tr.m_amount = -amount; is the one I changed in my patch to fix my problem for "sells". Need remove the negative sign. -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874
--- Comment #8 from allan ---
(In reply to Jeff from comment #6)
> Hi Łukasz,
>
> There is still a problem with the QIF import with your change. My test file
> also tested the case where the commission was greater than the proceeds from
> the sale (which can happen when trading options.) Your fix changed a "sell"
> trade that actually cost money into one that brought in money. The example
> in my test file was the "sell" of the "NFLX Aug 18 2012 110.0 Call". The
> price is 0.02, times 100 shares = 2.00. The commission is 10.77. So income
> of 2.00, outgo of 10.77 makes the total -8.77 (as shown in the U and T
> values in the QIF file). The cash account should decrease by 8.77. Your
> change turned that into a positive 8.77, and increased the cash account.
> This is admittedly a corner case, and I think I am the only KMM user that
> trades options because I have made a bunch of other changes to the KMM code
> to support that.
>
It would not have surprised me if your problem was caused by the same issue I
reported in https://bugs.kde.org/show_bug.cgi?id=362139 comment #7, with the
negative sign getting dropped.
However, the sign appears to be negative throughout the import process until -
Line 1564 in mymoneyqifreader.cpp()
"else if (action == "sell") {
d->st.m_listPrices += price;
tr.m_shares = -quantity;
tr.m_amount = -amount;
tr.m_eAction = (MyMoneyStatement::Transaction::eaSell);"
I don't profess to know anything about a sell where the commission is greater
than the proceeds of the sale. However, I think what happens in general is
that a sell is expected to be input with no sign, and therefore the amount
needs to be set to a negative value. So, in your case, as you have a negative
sign on input, that sign gets removed.
That seems to imply that the commission needs to negative in your case, and
reversing the signs appears to produce your expected result.
If this is all rubbish, just ignore it.
--
You are receiving this mail because:
You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #7 from allan --- (In reply to NSLW from comment #4) > Statement reader did it wrong also for CSV imports (see bug #361021) so I > made corrections to the code which seemed to help also QIF reader. Honestly > at that time I didn't know that investment statements are imported by > anything else than CSV and thanks to your bug and code review I see bigger > picture now. I proposed two next patches to statement reader which now I > must review myself because I already see mixed logic between QIF and CSV > imports, which somehow worked by now. > It would be nice if you could send me for testing an anonymized QIF file > with less frequent transaction types such as "dividends" etc. > > I don't know about OFX imports because valid ACCTTYPEs are only: CHECKING, > SAVINGS, MONEYMRKT, and CREDITLINE, and there are no investment ACCTTYPE, so > I assume OFX doesn't support that. Correct me if I'm wrong. The OFX specification 2.0.3 includes - "CHAPTER 13 INVESTMENTS OFX supports download of security information and detailed investment account statements including transactions, open orders, balances, and positions. " plus a lot more in detail. Allan -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #6 from Jeff --- Hi Łukasz, There is still a problem with the QIF import with your change. My test file also tested the case where the commission was greater than the proceeds from the sale (which can happen when trading options.) Your fix changed a "sell" trade that actually cost money into one that brought in money. The example in my test file was the "sell" of the "NFLX Aug 18 2012 110.0 Call". The price is 0.02, times 100 shares = 2.00. The commission is 10.77. So income of 2.00, outgo of 10.77 makes the total -8.77 (as shown in the U and T values in the QIF file). The cash account should decrease by 8.77. Your change turned that into a positive 8.77, and increased the cash account. This is admittedly a corner case, and I think I am the only KMM user that trades options because I have made a bunch of other changes to the KMM code to support that. So, to support my particular needs, I am going to undo your change in my local copy and restore my QIF importer change (which I submitted to the review board on Aug 15, 2015 but was never accepted). And still claim that the master branch does not fix my problem. (Though I hardly use QIF import anymore. I was using it to transfer my data from Quicken to KMM. I do have one investment account that doesn't support OFX and I use QIF for that one.) And I know that without your change, my OFX imports have been working. I don't have a quick and easy test for OFX import. But my review board submittal for the QIF importer change says I made the QIF importer data look just like the OFX importer data. Thus my concern in comment #3 about OFX testing with your change. I suspect OFX will work except for the case I explained in my first paragraph. Which probably nobody but me cares about :-) Jeff. -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #5 from Jack --- I import to investment accounts from OFX frequently, and it works just fine. (Well, mostly, but my problems are mainly in what my broker provides, not how KMM handles it, and I've complained about it in the past on the mailing list.) I can probably provide some example files, but I'd have to choose carefully for ones that don't show any of the problems. -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #4 from NSLW --- Statement reader did it wrong also for CSV imports (see bug #361021) so I made corrections to the code which seemed to help also QIF reader. Honestly at that time I didn't know that investment statements are imported by anything else than CSV and thanks to your bug and code review I see bigger picture now. I proposed two next patches to statement reader which now I must review myself because I already see mixed logic between QIF and CSV imports, which somehow worked by now. It would be nice if you could send me for testing an anonymized QIF file with less frequent transaction types such as "dividends" etc. I don't know about OFX imports because valid ACCTTYPEs are only: CHECKING, SAVINGS, MONEYMRKT, and CREDITLINE, and there are no investment ACCTTYPE, so I assume OFX doesn't support that. Correct me if I'm wrong. Is this bug solved for you then? -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 --- Comment #3 from Jeff --- Hi Łukasz, The git master branch does import my test file without any warnings. I had patched my copy of mymoneyqifreader.cpp to fix my problem. I undid that patch to test your fix. I'm a little concerned about the change to mymoneystatementreader.cpp. I didn't change it to fix my problem because the statement reader also has to handle csv and OFX imports. You have verified the csv import. Have you verified OFX imports? Jeff. -- You are receiving this mail because: You are watching all bug changes.
[kmymoney4] [Bug 351874] QIF import of investment buys and sells mishandles commissions
https://bugs.kde.org/show_bug.cgi?id=351874 NSLW changed: What|Removed |Added CC||lukasz.wojnilow...@gmail.co ||m --- Comment #2 from NSLW --- Hi Jeff, could you please check with KMM from git master branch to see if the problem persists. I import your QIF file and there are no warning triangles. Commissions seems to me correct either. Earlier I've got the same problem with CSV importer but I managed to patch KMM to solve that problem and to me it seems that inadvertently it solved this bug too. Regards Łukasz -- You are receiving this mail because: You are watching all bug changes.
