CSV Importer doesn't recognize sell operation during import. It recognizes Buy operation though. Reproducible: Always Steps to Reproduce: 1. Create your investing bank account, 2. File->Import->CSV... 3. choose Investing, give name to new CSV Importer template 4. Press Choose File and point my "test file.csv" (see attachment) 5. Field Separator: comma ; Text separator double quotes and go next, 6. assign columns accordingly (see attachment) and go next 7. at rows range just go next, 8. window appears to accept new securities so press just ok. 9. set Decimal Separator: comma 10. press Import CSV Actual Results: CSV Importer doesn't recognize my sell operation and asks what kind of operation is this. Expected Results: Recognize sell operation. 1) My locale is pl_PL 2) Buy operations are recognized, but only if their name is "Kupno" (Polish word for buy) and not "Buy" 3) Sell operations aren't recognized no matter if the name is "Sprzedaż" (Polish word for sell), "Sell" or "sell" 4) My CSV file is UTF-8
Created attachment 97694 [details] CSV Test File
Created attachment 97695 [details] Column Assignment
This does work for me, but I'll need to get back to you later with a bit more detail about what is required. Allan
After your step 10 above, the CSV Importer window displays the first transaction as an Invalid Transaction Type, because the importer does not understand Polish, so it gives you the opportunity to select what type of activity 'Sprzedaż' really is, in KMyMoney language. In the drop-down, you need to select Sell Shares, this being one of the activity types that match the values - quantity, price, amount - that are in that transaction. You should see now that the Type is Sell. As this transaction type involves money, you next need to enter the name of the checking/brokerage account you wish to use. The next transaction follows a similar path, with 'Kupno' as the activity type. So, for this you need to select Buy Shares. The remaining transactions follow the same course. I notice that the Buy transactions imported show as unbalanced, with a missing assignment of 6,00. It rather looks as though the fee is getting the wrong sign, which I'll need to have a look into. If any of that is unclear, please let me know. If you think the handbook needs expanding, also, please say how. Allan
(In reply to allan from comment #4) > After your step 10 above, the CSV Importer window displays the first > transaction as an Invalid Transaction Type, because the importer does not > understand Polish, so it gives you the opportunity to select what type of > activity 'Sprzedaż' really is, in KMyMoney language. In the drop-down, you > need to select Sell Shares, this being one of the activity types that match > the values - quantity, price, amount - that are in that transaction. You > should see now that the Type is Sell. As this transaction type involves > money, you next need to enter the name of the checking/brokerage account you > wish to use. The next transaction follows a similar path, with 'Kupno' as > the activity type. So, for this you need to select Buy Shares. The > remaining transactions follow the same course. > > I notice that the Buy transactions imported show as unbalanced, with a > missing assignment of 6,00. It rather looks as though the fee is getting > the wrong sign, which I'll need to have a look into. > > If any of that is unclear, please let me know. If you think the handbook > needs expanding, also, please say how. > > Allan Hi Allan, thanks for fast reply. Your description is good when it comes to Sell operations, but it is not so with Buy operations, because my Buy operations are recognized automatically and it seems that in this case KMyMoney understands Polish. Surprisingly all transactions are invalid if I've got Sell and Buy in CVS in English. Please read my additional information from the beginning. Besides wouldn't it be good if KMyMoney would ask only to identify unrecognized operation type? Łukasz
*Besides wouldn't it be good if KMyMoney would ask only once to identify unrecognized operation type?
> Hi Allan, > thanks for fast reply. Your description is good when it comes to Sell > operations, but it is not so with Buy operations, because my Buy operations > are recognized automatically and it seems that in this case KMyMoney > understands Polish. No, I'm afraid not! But, it reminds me that there is a bit more to this than I indicated. It's a while since I've been in this area. When importing investment files from different institutions, It was obvious that they had varying ideas as how to describe investment types. To allow for this, In the importer resource file, I've allowed for alternative versions to be added to a list for each investment type. I did not at that stage even consider non-English versions, but happily they can be added in the same way. I have no idea why 'Kupno' is recognised and 'Sprzedaż' is not. Both are foreign to me! All I can think is that the Polish translator has added 'Kupno' to that file, and I'm interested to know, if you would look for me. The file-name is csvimporterrc, and it should be found in your home directory, under /.kde/share/config/csvimporterrc (it could be .kde4....). Look for the section corresponding to the new profile you created, like [Profiles-your-profile], then within that look for 'BuyParam=xxx' and see if 'Kupno' appears. Similarly, there will be a 'SellParam' section. You could edit that and add 'Sprzedaż' minus quotes, and close and restart the csv plugin, to see if that helps. It might help also, to read the section in the user guide on importing csv files. > Surprisingly all transactions are invalid if I've got Sell and Buy in CVS in > English. Please read my additional information from the beginning. > > Besides wouldn't it be good if KMyMoney would ask only to identify > unrecognized operation type? > An excellent idea, and with the edited I mentioned above, that is how it should work. Let me know how that goes. Allan > Łukasz
Created attachment 97709 [details] csvimporterrc snippet I looked in csvimporterrc and found that 1) BuyParam=kup "kup" is an verb, while "kupno" is noun. Maybe KMyMoney detects "kupno" by regular expression. 2) SellParam= after setting this parameter to "Sprzedaż" my sell operations are recognized correctly, but why isn't that preset just like BuyParam? Maybe SellParam isn't preset because of last letter in "Sprzedaż", which is diacritic letter.
Created attachment 97712 [details] KMyMoney translation for sell Actually I am the sole translator for KMyMoney in Polish, and as I see words in kmymoney.po for parameters like BuyParam, SellParam, etc. all have "verb" as the context. According to that context "buy" is translated as "kup" and "sell" is translated as "sprzedaj" while in my case: "buy" should be translated as "kupno" and "sell" should be translated as "sprzedaż" In financial statement, my Bank gives me nouns and not verbs for sell and buy operations. For that reason maybe it makes sense to replace "buy" and "sell" with "BuyParam" and "SellParam". In English it would look following: BuyParam: buy, bought SellParam: sell, sold In Polish it would look following: BuyParam: kup, kupno SellParam: sprzedaj, sprzedaż As another suggestion, i think it is good to expand context from "verb" to "Action/Type as recognized by CSV Importer"
(In reply to NSLW from comment #8) > Created attachment 97709 [details] > csvimporterrc snippet > > I looked in csvimporterrc and found that > 1) BuyParam=kup > > "kup" is an verb, while "kupno" is noun. Maybe KMyMoney detects "kupno" by > regular expression. It's just "type.contains(*it, Qt::CaseInsensitive", so fairly loose to improve matching chance. Do you see any problem with that? > 2) SellParam= > > after setting this parameter to "Sprzedaż" my sell operations are recognized > correctly, but why isn't that preset just like BuyParam? Maybe SellParam > isn't preset because of last letter in "Sprzedaż", which is diacritic letter. I would have to throw that one back to you, as I didn't edit that file. Might you not have been the translator then? > As another suggestion, i think it is good to expand context from "verb" to "Action/Type as > recognized by CSV Importer" Perhaps that would make sense as you are a user as well as a translator? I'm not sure that a non-investment-wise translator might understand? So, apart from the fee query, are you now in business? Allan
(In reply to allan from comment #10) > (In reply to NSLW from comment #8) > > Created attachment 97709 [details] > > csvimporterrc snippet > > > > I looked in csvimporterrc and found that > > 1) BuyParam=kup > > > > "kup" is an verb, while "kupno" is noun. Maybe KMyMoney detects "kupno" by > > regular expression. > > It's just "type.contains(*it, Qt::CaseInsensitive", so fairly loose to > improve matching chance. Do you see any problem with that? It works with my Buy operations, but I would like it to work also with my Sell operations out of the box. I think that "type.contains(*it, Qt::CaseInsensitive)" is harmless here. > > 2) SellParam= > > > > after setting this parameter to "Sprzedaż" my sell operations are recognized > > correctly, but why isn't that preset just like BuyParam? Maybe SellParam > > isn't preset because of last letter in "Sprzedaż", which is diacritic letter. > > I would have to throw that one back to you, as I didn't edit that file. > Might you not have been the translator then? "Sprzedaż" is meant too look like this so no need to edit it. What I meant by above is: Could there be an sort of encoding problem in KMyMoney with that Polish letter, which in the end prevents positive recognition? > > As another suggestion, i think it is good to expand context from "verb" to "Action/Type as > > recognized by CSV Importer" > > Perhaps that would make sense as you are a user as well as a translator? > I'm not sure that a non-investment-wise translator might understand? As it is now was also misleading to me, to today although I'm an fresh KMyMoney user :) and translator, so maybe another suggestion will sound better to an layman: "Type of operation as in financial statement" > So, apart from the fee query, are you now in business? After manually editing csvimporterrc file, fee is imported just right for all operations. What do you mean by me being in business?
Created attachment 97870 [details] Patch to fix this and simillar issues Please revise it and apply to master branch.
(In reply to NSLW from comment #11) > (In reply to allan from comment #10) > > (In reply to NSLW from comment #8) > > > Created attachment 97709 [details] > > > csvimporterrc snippet > > > > > > I looked in csvimporterrc and found that > > > 1) BuyParam=kup > > > > > > "kup" is an verb, while "kupno" is noun. Maybe KMyMoney detects "kupno" by > > > regular expression. > > > > It's just "type.contains(*it, Qt::CaseInsensitive", so fairly loose to > > improve matching chance. Do you see any problem with that? > > It works with my Buy operations, but I would like it to work also with my > Sell operations out of the box. I'm afraid that may be a tall order. It will only work out of the box if the file being imported actually uses the exact same translation as that of the translator of the KMyMoney file. Unfortunately, what tends to be found is that different financial institutions use different terms to describe their transaction type, in fact sometimes changing within the same file. This is the reason I allowed for alternative terms to be added to the parameter list in the resource file. If the file being imported contains an unrecognised type, the user will be informed and given the chance to substitute with the standard KMyMoney transaction type. If this should happen more than the odd time, the user may add that unrecognised type to the appropriate list in the resource file. What I'm not sure about just now is whether, once the user has chosen to make a substitution, that new term is added to the resource file automatically. It's a few ears since I wrote that code. > I think that "type.contains(*it, Qt::CaseInsensitive)" is harmless here. > > > > 2) SellParam= > > > > > > after setting this parameter to "Sprzedaż" my sell operations are recognized > > > correctly, but why isn't that preset just like BuyParam? Maybe SellParam > > > isn't preset because of last letter in "Sprzedaż", which is diacritic letter. I know no Polish financial terms. If the Buy list contains a correct Polish term, then that I presume was added by the translator. A possible explanation is that at line 161 (in my version) of investprocessing.cpp the following appears - "m_buyList += i18nc("verb", "buy"); // some basic entries in case rc file missing". It may be that that was when the translation for Buy was added. However, the next line is similar for the Sell operation, and that was not translated apparently. I would suggest that you include both noun and verb versions for Sell in your resource file. > > > > I would have to throw that one back to you, as I didn't edit that file. > > Might you not have been the translator then? > > "Sprzedaż" is meant too look like this so no need to edit it. What I meant > by above is: Could there be an sort of encoding problem in KMyMoney with > that Polish letter, which in the end prevents positive recognition? > > > > As another suggestion, i think it is good to expand context from "verb" to "Action/Type as > > > recognized by CSV Importer" > > > > Perhaps that would make sense as you are a user as well as a translator? > > I'm not sure that a non-investment-wise translator might understand? > > As it is now was also misleading to me, to today although I'm an fresh > KMyMoney user :) and translator, so maybe another suggestion will sound > better to an layman: > "Type of operation as in financial statement" > That does sound more understandable, although it would have to be added to every transaction type. > > So, apart from the fee query, are you now in business? > > After manually editing csvimporterrc file, fee is imported just right for > all operations. What do you mean by me being in business? Apologies for that. I'm extremely impressed by the general level of knowledge of English shown by the users, and developers, and I forget myself sometimes and lapse into the vernacular. "being in business" is used to indicate that a problem has been solved and normal service may be resumed. I hope it didn't cause offence. So far as your suggested patch is concerned, I'm not sure of its purpose. The Subject is "[PATCH] Read operation's type explicitly as QStringList", and in it you add a number of QStringList definitions, which are already included in investprocessing.h, c. line 164. Oh, and just as a post script, I've added "in Polish" to the bug heading, as the import operation is otherwise as expected. Allan
(In reply to allan from comment #13) > What I'm not sure about just now is whether, once the user has chosen to > make a substitution, that new term is added to the resource file > automatically. It's a few ears since I wrote that code. That's worth checking. That might be a second bug. > I know no Polish financial terms. If the Buy list contains a correct Polish > term, then that I presume was added by the translator. A possible > explanation is that at line 161 (in my version) of investprocessing.cpp the > following appears - "m_buyList += i18nc("verb", "buy"); // > some basic entries in case rc file missing". It may be that that was when > the translation for Buy was added. However, the next line is similar for > the Sell operation, and that was not translated apparently. All terms are and were translated. > I would suggest that you include both noun and verb versions for Sell in > your resource file. I already have them and it works for me. > > As it is now was also misleading to me, to today although I'm an fresh > > KMyMoney user :) and translator, so maybe another suggestion will sound > > better to an layman: > > "Type of operation as in financial statement" > > > That does sound more understandable, although it would have to be added to > every transaction type. I think you're right, is there any problem for it to be added to every transaction type? > > > So, apart from the fee query, are you now in business? > > > > After manually editing csvimporterrc file, fee is imported just right for > > all operations. What do you mean by me being in business? > > Apologies for that. I'm extremely impressed by the general level of > knowledge of English shown by the users, and developers, and I forget myself > sometimes and lapse into the vernacular. "being in business" is used to > indicate that a problem has been solved and normal service may be resumed. > I hope it didn't cause offence. No, none at all. You've suggested me a workaround and it worked for me as expected. > So far as your suggested patch is concerned, I'm not sure of its purpose. > The Subject is "[PATCH] Read operation's type explicitly as QStringList", > and in it you add a number of QStringList definitions, which are already > included in investprocessing.h, c. line 164. I think that after my patch, every variable is defined anew locally, which I think is not clean way to do this and doesn't remove the source of the problem, but it works flawlessly. After the patch my csvimporterrc is filled out automatically with all the data whereas before the patch only BuyParam and ReinvdivParam were filled out. I tell you what, I'm going to see into the problem once again soon and I will try to find its source, cause I really want it to see it fixed. > Oh, and just as a post script, I've added "in Polish" to the bug heading, as > the import operation is otherwise as expected. I would say "in non-English" Łukasz
(In reply to NSLW from comment #14) > (In reply to allan from comment #13) > > What I'm not sure about just now is whether, once the user has chosen to > > make a substitution, that new term is added to the resource file > > automatically. It's a few ears since I wrote that code. > > That's worth checking. That might be a second bug. I'll check. > > > I know no Polish financial terms. If the Buy list contains a correct Polish > > term, then that I presume was added by the translator. A possible > > explanation is that at line 161 (in my version) of investprocessing.cpp the > > following appears - "m_buyList += i18nc("verb", "buy"); // > > some basic entries in case rc file missing". It may be that that was when > > the translation for Buy was added. However, the next line is similar for > > the Sell operation, and that was not translated apparently. > > All terms are and were translated. In the rc file snippet you attached, only Buy and Reinvdiv have content. > > > I would suggest that you include both noun and verb versions for Sell in > > your resource file. > > I already have them and it works for me. > > > > As it is now was also misleading to me, to today although I'm an fresh > > > KMyMoney user :) and translator, so maybe another suggestion will sound > > > better to an layman: > > > "Type of operation as in financial statement" > > > > > That does sound more understandable, although it would have to be added to > > every transaction type. > > I think you're right, is there any problem for it to be added to every > transaction type? Only in terms of bloat. However, if I create a single string, I can refer to that instead of using the full version. > > > > > So, apart from the fee query, are you now in business? > > > > > > After manually editing csvimporterrc file, fee is imported just right for > > > all operations. What do you mean by me being in business? > > > > Apologies for that. I'm extremely impressed by the general level of > > knowledge of English shown by the users, and developers, and I forget myself > > sometimes and lapse into the vernacular. "being in business" is used to > > indicate that a problem has been solved and normal service may be resumed. > > I hope it didn't cause offence. > > No, none at all. You've suggested me a workaround and it worked for me as > expected. > > > So far as your suggested patch is concerned, I'm not sure of its purpose. > > The Subject is "[PATCH] Read operation's type explicitly as QStringList", > > and in it you add a number of QStringList definitions, which are already > > included in investprocessing.h, c. line 164. > > I think that after my patch, every variable is defined anew locally, which I > think is not clean way to do this and doesn't remove the source of the > problem, but it works flawlessly. After the patch my csvimporterrc is > filled out automatically with all the data whereas before the patch only > BuyParam and ReinvdivParam were filled out. > I tell you what, I'm going to see into the problem once again soon and I > will try to find its source, cause I really want it to see it fixed. > > > Oh, and just as a post script, I've added "in Polish" to the bug heading, as > > the import operation is otherwise as expected. > > I would say "in non-English" Well, we don't know of any other problem, and it seems to depend on translation, so I think I'll stick with the current version. It can always be edited if needed in the future. > > Łukasz
(In reply to allan from comment #15) > > > I know no Polish financial terms. If the Buy list contains a correct Polish > > > term, then that I presume was added by the translator. A possible > > > explanation is that at line 161 (in my version) of investprocessing.cpp the > > > following appears - "m_buyList += i18nc("verb", "buy"); // > > > some basic entries in case rc file missing". It may be that that was when > > > the translation for Buy was added. However, the next line is similar for > > > the Sell operation, and that was not translated apparently. > > > > All terms are and were translated. > > In the rc file snippet you attached, only Buy and Reinvdiv have content. That's right and that also shows that KMM has bug inside. Please see translation progress of KMM to Polish to know that it is translated fully http://l10n.kde.org/stats/gui/trunk-kf5/team/pl/extragear-office/ Please read also what I wrote you about the bug I'm seeing > > After the patch my csvimporterrc is > > filled out automatically with all the data whereas before the patch only > > BuyParam and ReinvdivParam were filled out. And finally please see into code of investprocessing.cpp BuyParam and ReinvdivParam are assigned like this >>>>>>>>>>>>>>>>>>>>>>>> QStringList list = profilesGroup.readEntry("BuyParam", QStringList()); if (!list.isEmpty()) { m_buyList = list; } >>>>>>>>>>>>>>>>>>>>>>>> whereas other parameters and it includes SellParam is assigned like this >>>>>>>>>>>>>>>>>>>>>>>> m_sellList = profilesGroup.readEntry("SellParam", QStringList()); >>>>>>>>>>>>>>>>>>>>>>>> and that's the reason why you don't see SellParam in rc file and not the missing translation.
Created attachment 97970 [details] Do not fetch from csvimporterrc if it's empty Patch to fix this and similar issues. Please revise it and apply to master branch.
Created attachment 98417 [details] Fill parameters of new investment profiles with translated strings Additionally to previous patch, I recommend applying also this patch. It in some kind moves [Profiles-New Profile###] template from /usr/share/config/csvimporterrc to KMyMoney code, thus making it easy translatable. Please revise it and apply to master branch.