Skip to content
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

ENH implement plot_monthly_returns_timeseries in plotting.py to rende… #195

Merged
merged 3 commits into from
Nov 5, 2015

Conversation

justinlent
Copy link
Contributor

…r monthly bars representing monthly returns of an algo

…r monthly bars representing monthly returns of an algo
@justinlent
Copy link
Contributor Author

I want to add this plot as a default in tears.create_returns_tearsheet() as well. But might make sense to do that as a separate PR after this one is merged, along with after this one: #189

I looked at where it would make sense to add it to the tearsheet in tears.create_returns_tearsheet(), and it feels like it would be a bit of an intrusive modification given all the inter-connectedness of the plots in the figure/grid spec

@justinlent
Copy link
Contributor Author

Here's an example of what this feature looks like when rendered on an algo's returns:

image

@justinlent
Copy link
Contributor Author

@twiecki let me know if you think we can merge this. It only involves additions to the code base, and no modifications to anything existing, so pretty low risk. Let me know if it looks good to you.


# Generate month-year labels for the x-axis corresponding to
# the returns plotted
date_labels = [str(i[1]) + ' - ' + str(i[0]) for i in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with the default labels of this plot? I suppose there's too many? How about just thinning those. The current approach feels a bit brittle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep that's right, the defaults are super ugly because it prints the month/day/year of each rendered monthly bar and the plot becomes illegible if more than ~30 bars. 
As well, the defaults include the last business day of the month in each label (which just adds to the cluttered visual because the last business day of each month jumps around, 29, 30, 31, etc). My opinion is that including the day in the label is not really proper when labeling a monthly value, as well.
Pandas does not have a monthly frequency unit so I could not up-sample the DateTimeIndex to make it more elegant (which would have been best).
Given the above, and my desire to have an easily interpreted plot, how would you suggest improved pruning of the labels? I actually adapted my implementation from an accepted StackOverflow response
Sent from iPhone

_____________________________

From: Thomas Wiecki notifications@github.com
Sent: Wednesday, November 4, 2015 4:10 AM
Subject: Re: [pyfolio] ENH implement plot_monthly_returns_timeseries in plotting.py to rende… (#195)
To: quantopian/pyfolio pyfolio@noreply.github.com
Cc: Justin Lent justinlent@gmail.com

In pyfolio/plotting.py: > + Returns> + -------> + ax : matplotlib.Axes> + The axes that were plotted on.> + """> +> + if ax is None:> + ax = plt.gca()> +> + monthly_ret_table = timeseries.aggregate_returns(returns, 'monthly')> + monthly_ret_table = monthly_ret_table.reset_index()> + monthly_ret_table.columns = ['year', 'month', 'returns']> +> + # Generate month-year labels for the x-axis corresponding to> + # the returns plotted> + date_labels = [str(i[1]) + ' - ' + str(i[0]) for i in

What's wrong with the default labels of this plot? I suppose there's too many? How about just thinning those. The current approach feels a bit brittle.


Reply to this email directly or view it on GitHub.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@twiecki ...Actually, my bad, looks like Pandas does support a Monthly unit, under the concept of a Period. so timestamps can be converted to periods to drop the concept of a "day" in monthly data and have plots generated properly. I'll go ahead and re-implement...  Ohhh Pandas, so convoluted sometimes...

Sent from iPhone

On Wed, Nov 4, 2015 at 4:10 AM -0800, "Thomas Wiecki" notifications@github.com wrote:

In pyfolio/plotting.py:

  • Returns

  • ax : matplotlib.Axes
  •    The axes that were plotted on.
    
  • """
  • if ax is None:
  •    ax = plt.gca()
    
  • monthly_ret_table = timeseries.aggregate_returns(returns, 'monthly')
  • monthly_ret_table = monthly_ret_table.reset_index()
  • monthly_ret_table.columns = ['year', 'month', 'returns']
  • Generate month-year labels for the x-axis corresponding to

  • the returns plotted

  • date_labels = [str(i[1]) + ' - ' + str(i[0]) for i in

What's wrong with the default labels of this plot? I suppose there's too many? How about just thinning those. The current approach feels a bit brittle.


Reply to this email directly or view it on GitHub.

@justinlent
Copy link
Contributor Author

@twiecki I re-implemented using PeriodIndex - much cleaner implementation for sure. As well rendering some yearly boundary dashed-lines for viewing convenience.
image

@twiecki
Copy link
Contributor

twiecki commented Nov 5, 2015

@justinlent Looks great!

twiecki added a commit that referenced this pull request Nov 5, 2015
ENH implement plot_monthly_returns_timeseries in plotting.py to rende…
@twiecki twiecki merged commit 95f319c into master Nov 5, 2015
@twiecki twiecki deleted the monthly_returns_timeseries branch November 5, 2015 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants