Add detailed monthly returns heatmap with drawdown information #45
Conversation
…early return & drawdown information. Example: https://imgur.com/a/PsKjhWy - Implement new monthly_returns_detailedview function in wrappers.py - Add corresponding monthly_heatmap_detailedview function in core.py
Add detailed monthly returns heatmap with drawdown information
|
My review is in progress 📖 - I will have feedback for you in a few minutes! |
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
I have reviewed your code and found 1 potential issue. To discuss my individual comments that I have added, tag me in replies using @korbit-ai.
Please react with a 👍 to my comments that you find helpful and a 👎 to those you find unhelpful - this will help me learn and improve as we collaborate.
quantstats_lumi/_plotting/core.py
Outdated
| def monthly_heatmap_detailedview( | ||
| returns, | ||
| benchmark=None, | ||
| grayscale=False, | ||
| figsize=(14, 6), | ||
| annot_size=10, | ||
| returns_label="Strategy", | ||
| fontname="Arial", | ||
| savefig=None, | ||
| show=True, | ||
| ): |
There was a problem hiding this comment.
The monthly_heatmap_detailedview function is quite complex and lengthy, which can impact readability and maintainability. To improve this, consider breaking it down into smaller, more focused functions. This could involve separating the calculations of returns and drawdowns, as well as the heatmap generation, into their own functions. This will make the code easier to understand and maintain over time.
|
Max drawdown should not be any time period at all, it should be the biggest
drop the strategy had irrespective of how long it took for the drop to
happen. Isn’t that how it is now?
Robert Grzesik 347-635-3416
…On Wed, Jun 26, 2024 at 12:26 PM hygarlic ***@***.***> wrote:
Another thing, in the left margin, return is for the whole year, but MDD
is only the worst monthy return. It should be instead the maxDD for the
whole year.
—
Reply to this email directly, view it on GitHub
<#45 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIYQKYD6ZGIH2SYKE5WL6LZJLTTLAVCNFSM6AAAAABJ5LIJ2SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJSGEYTKMZTGY>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
Thank you for your feedback. I'll adjust the MDD to show the maximum drawdown for the entire year, matching the annual return figure. |
|
I don’t think the max drawdown should be adjusted at all. The max drawdown
is not over a month, a year or anything else. It is intended to be the
maximum loss, no matter the time period. For example, the SPY had a max
drawdown recently of around 30% which took almost two years.
As for the monthly max dd, which figures do you mean exactly? And what is
the issue here?
Robert Grzesik 347-635-3416
…On Wed, Jun 26, 2024 at 7:31 PM Taewan Kim ***@***.***> wrote:
Thank you for your feedback. I'll adjust the MDD to show the maximum
drawdown for the entire year, matching the annual return figure.
Regarding the monthly maxDD calculation I am going to fix code especially
for October 2023 to correct the discrepancy you noted.
—
Reply to this email directly, view it on GitHub
<#45 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIYQKYAWPO6SGC2NG55YC3ZJNFMDAVCNFSM6AAAAABJ5LIJ2SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJSG44DMNJSGQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
…onth-end basis - Ensures consistency between monthly returns and drawdowns calculations - Addressed feedback: - Removed "Year" and "Month" labels from axes - Updated title format - Adjusted layout - Added 'YTD' label to clarify year-to-date metrics on left margin
|
I've addressed most of the comments in the feedback. When I checked the returns file and looked at the period from the end of September to October 2023, I found that the return calculated from the last trading day of September to the last trading day of October is -7.67%, which is why it's displayed that way. However, the monthly drawdown was shown as -6.37% because it was based on October 1st to 31st. I've modified the function to calculate the monthly drawdown on a month-end to month-end basis, consistent with the returns calculation. I've also implemented the other suggested changes from the feedback.
|
|
I agree. I don't think this feature is particularly necessary for a basic HTML report. This code was created to view monthly drawdowns separately when generating specific plots. |
|
I've been making several attempts and have made some progress, but I'm still encountering a couple of issues:
I've moved the YTD label to the right-aligned position, which matches the reference image. |
for i in range(pivot_returns.shape[0]):
for j in range(pivot_returns.shape[1]):
cell = ax.get_children()[i * pivot_returns.shape[1] + j + 1]
return_color = cell.get_color()
monthly_dd_color = 'white' if return_color == 'w' else 'black'
ax.text(j + 0.5, i + 0.55, annot_drawdowns.iloc[i, j],
ha='center', va='top', fontsize=annot_size * 0.8, color=monthly_dd_color)When I added this code, it produced a plot that looks correct as shown below. I'm still unsure how to apply different font sizes to yticklabels in seaborn, particularly making the drawdown percentage smaller than the year and return percentage in the YTD section.
|
- Modify YTD label positioning - Set both monthly and yearly drawdown font sizes to 80% of return font size - Change basic annot_size to 11
|
|
|
For the existing white font tile, I set it to the gainsboro color because if I changed this colors dimgray, it would look bad on certain tiles. The following plot shows an example with all monthly dd set to dimgray.
|
|
It seems like () is needed for negative numbers like in financial statements, but I don't think it's necessary since there is an explicit - sign. The version without the () looks a bit cleaner, though. |
- add monthly_dd_font_rate, annual_dd_font_rate argument, use can edit both - increase default annual_dd_font_rate to 0.9
|
|
The cumulative returns of the strategy I used show many empty spaces without % because the period starts from 2019, making it look sparse. However, as you mentioned, if the period gets longer, it look strange with too many %. |
…n auto-detected size and a user-specified rate. - Add `return_font_rate` argument. - Improvements to the heatmap rendering process, including better handling of null values and grid removal. - Change gainsboro drawdown color to white for better visibility on orange tiles
|
Regarding the issue of overlapping returns and drawdowns in the YTD section for long-term data like BTC, I experimented with reducing the font size when overlaps occur. However, this approach led to two problems: the font became too small compared to the returns in the central heatmap, and overall readability was significantly compromised. Instead of reducing font size of it, I think it's better that users adjust the figsize argument to accommodate longer time periods. This approach maintains readability while allowing for more data to be displayed clearly. At this point, I believe this is the most suitable solution, as it gives users the flexibility to optimize the display for their specific data range.
|
- Adjust annual_dd_font_rate to 0.8 - Adjust vertical position of yearly return & dd by 0.05 up.
|
I realized that sns.heatmap doesn't automatically adjust the font size, but simply uses rcParams['font.size']. So I rolled back to the previous format. I kept the feature where users can either use annot_size as is or adjust it with return_font_rate. |
|
|
|
Thanks for the suggestions. I propose merging this PR for now. I'll create a new pull request for these improvements for future implementation when I have more bandwidth, including your suggestions and an update of the original heatmap. |
|
this is a great addition. any ideas when this can get merged? |
|
I will take a look at this soon and merge this + any other PRs. Been
injured the past week so I’m running behind
Robert Grzesik 347-635-3416
…On Wed, Jul 10, 2024 at 11:06 AM Brian ***@***.***> wrote:
this is a great addition. any ideas when this can get merged?
—
Reply to this email directly, view it on GitHub
<#45 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIYQK5S7NXB3L766OJAD4LZLVEXBAVCNFSM6AAAAABJ5LIJ2SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRQG43DANRSGA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
this is awesome @twkim112 ! great work! merging now |
|
This is now deployed in v0.3.2! 🎉 |
|
I'll create a PR in the next few days! |
|
Okay I'll try adding the cbar option to the original heatmap and see if that affects the width |
Key changes:
monthly_returns_detailedviewfunction in wrappers.pymonthly_heatmap_detailedviewfunction in core.pyThis new visualization provides a comprehensive view of strategy performance, allowing users to quickly identify patterns, strong/weak periods, and the relationship between returns and drawdowns over time.