-
-
Notifications
You must be signed in to change notification settings - Fork 483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add realized dividends to gross performance #3857
base: main
Are you sure you want to change the base?
Add realized dividends to gross performance #3857
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for your contribution, @dandevaud! 🙂
While reviewing, I was surprised that no test had to be adjusted with your PR. I have therefore extended the only dividend test (#3891). Can you please rebase here?
Ideally, the dividend performance should be listed here separately and also included in the overall performance:
What do you think?
- Dividends will be handled like sell-orders performancewise This might be one approach to solve issue ghostfolio#3233
21fe5c7
to
6f57439
Compare
Thanks @dtslvr I updated the teste accordingly. We can list the dividend separately, but I would already expect it in the "Absolute Asset Performance". There the "Change" and "Change with currency effect" should be change to something like "Total Return" and "Total Return with currency effect". Let me know what you think and if I should adapt the overal performance as well. |
Hi @dtslvr Please excuse my late reply. Sounds better, but instead of dividend income we should use something like Realized Profit/Loss, which includes sells as well. Edit: Complicated in the sense of what to use as basis. It should be the same over all 3 values and this would be total investments without reducing it by the sells. |
Hi @dtslvr Again please excuse my late reply, had some busy weeks. I revisted the theoretical performance calculation, meaning I have not yet checked the code on how to implement it as I wanted your inputs first. The idea of splitting up the absolut net asset performance good, I would nonetheless go with realized and unrealized gains/losses. I currently face a problem with the relative performance, respectively the cost basis to use.
Calculating the P/Ls would lead to:
For the relative performance we need to have a cost basis to calculate against.
The problem is the unrealized P/L does not correctly refelct the price change of the underlying asset. Whilst the asset price doubled, the unrealized P/L only has a performance of 75%. I checked yahoo finance how they handle it and the use different cost basis for the different P/L calculation, but do not show a total performance value.
I would go with the first approach not correctly reflecting the price change, as it accounts for all performance of a portfolio. Whats your take on this? Or would you even choose a totally different approach? Many thanks for your feedback and kind regards |
Hi @dtslvr
I saw there was an issue raised with dividends lowering the performance (#3233)
I added logic to handle dividends like sell-order for performance calculation.
I would say from economical perspective this makes most sense, as when dividends are paid the stock price will lower by the dividend amount.
Moreover, I extracted the sellOrder and dividend handling in a separate method for readability