portfolio-performance / portfolio

Track and evaluate the performance of your investment portfolio across stocks, cryptocurrencies, and other assets.
http://www.portfolio-performance.info
Eclipse Public License 1.0
2.87k stars 592 forks source link

Anomalies Loading Payments by CSV #3380

Open flywire opened 1 year ago

flywire commented 1 year ago

An obvious question is, why would anyone try and load payments by csv using anything other than the Ticker Code without the extension? I assume this arose through the need to look up Shares and inadvertently the Ticker Code with the extension was also returned and used. Anyway, it fails.


Describe the bug

Field usage is inconsistent when loading payments with CSV and the loader doesn't make best use of available data.


To Reproduce

Follow https://forum.portfolio-performance.info/t/import-csv-file/17123 to create the demo portfolio but don't load the dividends.

From https://github.com/buchen/portfolio/pull/3378#issuecomment-1574929831

This is something I don't understand. I use Yahoo as the quote provider and it uses .AX for the .ASX exchange. Initially, I enter a security without the exchange suffix, and then when the Quote Feed Exchange is selected as say FLT.AX (Australian Stock Exchange (ASX)) the .AX is appended to the symbol.

View, Securities, select REA, Transaction, Dividend - Note Security is actually Security Name which must be used to select it:

image

DemoDividends.csv

Paid,Code,Shares,Dividend,Franking,Total
23/03/21,REA,64,37.76,16.19,53.95
15/04/21,RIO,89,460.23,197.24,657.47
2/07/21,ALL,459,68.85,29.51,98.36

This example loads without problem (but I'm fairly sure it is not always the case).

These examples don't load income to the existing security, they create a new security and load it into that:

DemoDividends_AX.csv

Paid,Code,Shares,Dividend,Franking,Total
23/03/21,REA.AX,64,37.76,16.19,53.95
15/04/21,RIO.AX,89,460.23,197.24,657.47
2/07/21,ALL.AX,459,68.85,29.51,98.36

DemoDividends_ASX.csv

Paid,Code,Shares,Dividend,Franking,Total
23/03/21,REA.ASX,64,37.76,16.19,53.95
15/04/21,RIO.ASX,89,460.23,197.24,657.47
2/07/21,ALL.ASX,459,68.85,29.51,98.36

image

image


~Expected~ Desired behavior

There a two issues with terms:

  1. Different terms are used to describe the same data item in different parts of PP (possibly only short names used for column headings)
  2. The term Bank seemingly refers to Broker. It follows that it wouldn't be appropriate to have a bank pdf parser. Typical eg:

Savings Bank Statement

Date,Description,Debit,Credit,Balance
23/03/2021,Rea Payment Mar21/30044229,,$37.76,$200.23
  1. It's annoying the locale date format (at least day before month part ) is not recognised
  2. Ticker Symbol is not a field but it is fairly easy to derive
  3. Shares are unknown in the bank statement but PP knows this information in the Securities account
  4. Franking (see previous examples above) is a tax credit (ie income) yet PP does not allow -ve tax costs to be input (if it did a different payment data source would be required)

Desktop:

buchen commented 1 year ago

Hi @flywire,

I checked this issue. I think it is fixed now (as part of the Stake PDF Importer). At least I was not able to reproduce it.

See the screen shot. The security has a ticker of RIO.AX. The CSV has a ticker of RIO.ASX. As you can see from the second screenshot, the security is not created afresh, but the existing one is used (beside two new creations)

Bildschirmfoto 2023-06-25 um 18 26 08 Bildschirmfoto 2023-06-25 um 18 26 15

Can you confirm?

flywire commented 1 year ago

I think it is fixed now... Can you confirm?

Hmm, you did not follow the reproduce instructions. The imported security created from the dividend when the security already exists is a symptom of the problem.

Follow https://forum.portfolio-performance.info/t/import-csv-file/17123 to create the demo portfolio but don't load the dividends.

Anyway, as you say that part of the issue is fixed now. :tada:


What are your thoughts about loading a generic Bank Statement? A CSV loader could resolve points 1 & 2 but points 3 & 4 seem to be limited by PP.

buchen commented 1 year ago

@flywire apologies, you are right - I was just looking at one point out of many points...

Let's unpack the points one by one:

  1. It's annoying the locale date format (at least day before month part ) is not recognised

The algorithm to guess the format is very basic. 🫣 In particular, the guessing depends on the first value of the first line to be imported. In the screenshots below, I am running with the en_US locale and have selected the sample file. And guessing the order of the month and day in something like 23/01/01 is difficult.

Bildschirmfoto 2023-08-10 um 18 14 19 Bildschirmfoto 2023-08-10 um 18 14 36

I am trying to understand in which direction to improve: does it require to look at more values? Can you provide more examples? We could take into account the locale and use the default short format of the locale.

  1. Ticker Symbol is not a field but it is fairly easy to derive

I believe this point is the one that has been fixed (= matching securities by ticker symbol, not including the market).

  1. Shares are unknown in the bank statement but PP knows this information in the Securities account

Just to make sure I understand you correctly:

When importing dividends, the number of shares might not be part of the CSV file. PP could go and lookup the holdings of this security at that point in time and fill in the number of shares. If the security is held in multiple securities account, it would try to use the one which has configured as reference account the cash account which is used to record the dividends.

  1. Franking (see previous examples above) is a tax credit (ie income) yet PP does not allow -ve tax costs to be input (if it did a different payment data source would be required)

If I understand "franking" correctly, it means you, as an investor, get credited taxes that the company has paid on the profit. (I am not aware of something equivalent in Germany).

To be honest, I am not sure how to record this in PP. We have a separate transaction "tax refund". It is used if you paid taxes at timepoints A and then later it turns out you get (some) of the taxes back at point B. But overall, taxes are something that the users pays. And usually I pay more taxes than I get back in a given year (which also means PP can display this in pie charts, etc., because negative values are very hard to display as pie chart).

I see the franking as a "incentive" to pay dividends. But the dividends are earnings. It is profit. Does it matter that some of the profits take a detour via the state coffers? Therefore my gut feeling is to record as dividends.

(Once we know as what to record, we can discuss how to support this in the CSV importer).

buchen commented 1 year ago

It's annoying the locale date format (at least day before month part ) is not recognised

I see the issue now in the sample as well. I was blind... Let me check how to improve the pre-selection of the date format

flywire commented 1 year ago
  1. It's annoying the locale date format (at least day before month part ) is not recognised

A. Use locale order B. Do not show separators in dropdown list, only dmy order C. Consider [maybe] alphabetic month (Mar, March)

  1. Ticker Symbol is not a field but it is fairly easy to derive

I believe this point is the one that has been fixed (= matching securities by ticker symbol, not including the market).

Date,Description,Debit,Credit,Balance
23/03/2021,Rea Payment Mar21/30044229,,$37.76,$200.23
  1. Shares are unknown in the bank statement but PP knows this information in the Securities account

When importing dividends, the number of shares might not be part of the CSV file. PP could go and lookup the holdings of this security at that point in time and fill in the number of shares.

Correct, essentially the manual process.

If the security is held in multiple securities account, it would try to use the one which has configured as reference account the cash account which is used to record the dividends.

Even better, [pdf/csv] importer should recognise accountId --> Account identification. PP needs to store account id for each Deposit/Security account and the entity for each.

  1. Franking (see previous examples above) is a tax credit (ie income) yet PP does not allow -ve tax costs to be input (if it did a different payment data source would be required)

Firstly, apologies for pushing this issue so hard but it is fundamental and essential for Australian users. It is a feature of the two commercial alternatives in Australia, one available as crippleware. I recognise the objectives of not catering for tax reports and special needs of other countries but I think this can be handled with a tweak.

See discussion in https://forum.portfolio-performance.info/t/dividends-with-imputation-credits/17148/14 and #3450 but I'd like to discuss further.

Your understanding is correct but I'd like to focus on alternate approaches to two things.

PP can display this in pie charts

If tax is negative it is income rather than an expense, so show it appropriately in charts. Can you provide specific example(s) of issues?

Therefore my gut feeling is to record as dividends.

No, don't do that, that would really limit PP usefulness in Australia.

Taxes can change over time but with a constant (30%) Franking Credit, the Franked and Unfranked amount can be calculated from the Total amount. *

ie users get the info they need for tax reporting as a by-product.


Note for future reference. In Australia, if you do not supply a Tax File Number institutions deduct withHoldingTax (always creditableWithHoldingTax ??) which is part income and normally refunded in your tax return.

buchen commented 1 year ago

@flywire - I have improved the pre-selection of the data format (it was a quick win)

Just to let you know: I am on the road for the next couple days. But I will revisit the "franking discussion"

flywire commented 11 months ago

I don't agree the date issue has been fixed but with the above commit, the issue never seems to occur. :smile:

I will revisit the "franking discussion"

bump