ENH Pyfolio integration #227

luca-s · 2017-12-07T22:54:33Z

Issue #225

Added performance.create_pyfolio_input, which create input suitable for pyfolio. Intended use:

factor_data = alphales.utils.get_clean_factor_and_forward_returns(factor, prices)

pf_returns = alphales.performance.create_pyfolio_input(factor_data,'1D',
                     long_short=True,
                     group_neutral=False,
                     quantiles=None,
                     groups=None)

pyfolio.tears.create_returns_tear_sheet(pf_returns)

Also, I greatly improved the function that computes the cumulative returns as that is the base on which the pyfolio integration feature is built on. Mainly I removed the legacy assumptions which required the factor to be computed at specific frequency (daily, weekly etc) and also the periods had to be multiple of this frequency (2 days, 4 days etc). The function is now able to properly compute returns when there are gaps in the factor data or when we analyze an intraday factor.

luca-s · 2017-12-07T23:00:00Z

Oh, I forgot. To properly compute the portfolio returns I needed to know the length of the periods. Currently they are just integers and it is not known the units of those integers. So I changed the period column names from a simple integer to a pd.Timedelta string representation (1D, 5H, etc)

luca-s · 2017-12-07T23:13:09Z

I'll go through pep8 issues tomorrow.

luca-s · 2017-12-08T13:01:28Z

Ok, finished with pep8 issues. This PR is ready

twiecki · 2017-12-08T13:38:53Z

alphalens/performance.py

        Period over which to calculate the turnover
+        if 'period' is a string it must follow pandas.Timedelta constructor
+        format (.e.g '1 days', '1D', '30m', '3h', '1D1h', etc)


.e.g -> e.g.

twiecki · 2017-12-08T13:41:17Z

This looks great!

luca-s · 2017-12-08T16:48:40Z

The PR is ready to be merged but I expect some issues with pyfolio as I haven't done extensive testing. Anyway I don't expect regressions with existing functionalities and I actually expect better compatibility with non-daily factors and event studies.

luca-s · 2017-12-12T10:48:47Z

@twiecki I am going to merge this if you agree

twiecki · 2017-12-12T11:35:43Z

alphalens/utils.py

+
+def timedelta_to_string(timedelta):
+    """
+    Utility that converts a pandas.Timedelta to a string rappresentation


representation

twiecki · 2017-12-12T11:36:08Z

alphalens/utils.py

+    return columns[valid_columns]
+
+
+def timedelta_to_string(timedelta):


Does that really not exist inside pandas?

Unfortunately no, the one in pandas gives back the verbose string. e.g. '1 days 00:00:00', which doesn't suit the column headers

twiecki · 2017-12-12T12:32:04Z

Does this change existing behavior?

luca-s · 2017-12-12T12:42:04Z

No, all scenarios that were working before are working now + the current behaviour does a better job in handling factors with gaps (when values are not computed every day, e.g. event study) and intraday factors.

The only user level visible change is the period column names that are now named "1D", "3D" (1 day, 3 days or "1H", 3H" for 1 hour, 3 hours, etc. ) instead of "1", "3".

twiecki · 2017-12-12T13:12:09Z

OK, great. And users can now also have arbitrary periods, like hours.

I haven't reviewed the code carefully but I'm fine merging this as an experimental feature.

twiecki · 2017-12-12T13:12:39Z

Actually, let's see if @richafrank has someone who could do a code review.

luca-s · 2017-12-12T13:31:32Z

Actually arbitrary periods are possible even without this PR, for example a daily factor with 1 hours period or a weekly factor (computed on Monday for example) with a period of 3 days. With this kind of factors the results are computed correctly even though the turnover analysing is not so interesting, but the cumulative returns are wrong and this prevent the pyfolio analysis, this is why I fixed it in this PR and, as I was already fixing the code, I made sure the cumulative returns is correct with factors not computed at a fixed frequency, e.g. event study

abhijeetkalyan · 2017-12-13T20:46:21Z

@luca-s @twiecki this looks neat! I had a couple questions around the UX:

The name confused me a bit. When I saw create_pyfolio_input, I expected it to have positions and transactions as well. Since the function's output is the return stream, should we change its name to clarify that it's providing only returns, and not all of the other inputs that pyfolio consumes?
If this is still an experimental feature as @twiecki mentioned in ENH Pyfolio integration #227 (comment), should we add some kind of warning to inform users of that fact?

luca-s · 2017-12-13T21:27:46Z

You are right @abhijeetkalyan , the name is confusing a little. The reason is that I initially thought to return positions and transactions as well but I haven't managed to do that yet. I am actively working on this so my plan is to improve create_pyfolio_input to make it returns positions and transactions in another PR. I hope to push that PR soon together with an example NB. If I manage to do that before we create a new tag I would avoid the warning, otherwise if we create a tag and release a new version of Alphalens before I finish working on this feature I will add a warning message. Sounds like a plan?

luca-s · 2017-12-14T10:17:44Z

I added an example NB and also I made create_pyfolio_input returns the benchmark returns too (as the factor universe mean returns).

luca-s · 2017-12-14T13:18:10Z

I also added a warning in create_pyfolio_input docstring:

WARNING: this API is still in experimental phase and input/output
paramenters might change in the future

I believe this PR is now ready to be merged

prsutherland

Wow, this is awesome! I'm a big fan of the movement from integers to Timedelta strings for period. Is it possible to keep the default values on the period parameters? I'm not sure what makes sense as a default period, '1D', maybe?

Also, it looks like https://github.com/quantopian/alphalens/blob/master/alphalens/examples/tear_sheet_walk_through.ipynb uses many of the methods that have interface changes to. The notebook should probably be updated to use the new form of the methods.

prsutherland · 2017-12-13T23:05:27Z

alphalens/performance.py

@@ -68,7 +68,7 @@ def src_ic(group):
        grouper.append('group')

    ic = factor_data.groupby(grouper).apply(src_ic)
-    ic.columns = pd.Int64Index(ic.columns)
+    ic.columns = pd.Index(ic.columns)


Since ic.columns is already an index, this line and https://github.com/quantopian/alphalens/pull/227/files#diff-39e40ed256ddc7619de160850cb6f6a2R125 can probably be removed.

I guess you are right :)

prsutherland · 2017-12-14T16:22:22Z

alphalens/utils.py



-def std_conversion(period_std):
+def std_conversion(period_std, one_period_len):


one_period_len isn't in the docstring. Looks like it should be a string in a Timedelta format.

you are right!

luca-s · 2017-12-14T21:34:38Z

@prsutherland thanks for reminding me about the walk through NB, I completely forgot about that. Regarding the default values on the period parameters I am not sure... If I find a sensible default I would restore them.
The movement from integers to Timedelta strings for period was needed to carry information that otherwise would have been lost but some thorough testing is now required because this change could lead to some hidden and non-obvious bugs. Sometimes I think get_clean_factor_and_forward_returns should return an object where we can store all the information we want together with factor_data instead of trying to save the information we need inside the DataFrame column names...but for now let's keep it this way.

luca-s · 2017-12-19T21:02:55Z

Unless there are other comments I am going to merge this PR soon. I only need to finish updating the tear_sheet_walk_through NB.

Most of Alphalens functions are now factor frequency agnostic (the factor can be computed daily, weekly, randomly) and they don't assume forward returns periods to be multiple of the frequency at which a factor is computed, but there are still some functions that rely on those assumptions. This commit addresses those limitations. The main function which requires a fix is 'cumulative_returns', which assumes returns to be computed at a specific frequency (daily, weekly, etc) without gaps (e.g. an event which can happend randomly doesn't work well with current implementation) and the return periods must be multiple of factor computation frequency. In order to remove those limitation from 'cumulative_returns' the period length for which the returns are computed is required and for this reason is now included in the forward columns name, which now contains the period lenght (e.g. 1 day, is now '1D', 3 days is now '3D', 2 hours '2h' and so on). Other small fixed are included in this commit which are related to the removal of the above assumptions. This commit also add tests for performance.cumulative_returns.

Make sure the NB reflect the new API

twiecki · 2017-12-19T22:05:45Z

Looks great, @luca-s. Thanks!

luca-s mentioned this pull request Dec 7, 2017

Integration with pyfolio and Quantopian's new Risk Model #225

Closed

luca-s force-pushed the pyfolio_integration branch 2 times, most recently from 9ce1247 to 2511db4 Compare December 8, 2017 12:52

twiecki reviewed Dec 8, 2017

View reviewed changes

luca-s changed the title ~~Pyfolio integration~~ ENH Pyfolio integration Dec 10, 2017

luca-s force-pushed the pyfolio_integration branch 5 times, most recently from ac5e19d to 7090c26 Compare December 12, 2017 09:41

twiecki reviewed Dec 12, 2017

View reviewed changes

luca-s force-pushed the pyfolio_integration branch from 7090c26 to b0e8e19 Compare December 12, 2017 12:15

luca-s force-pushed the pyfolio_integration branch from b0e8e19 to 969235d Compare December 13, 2017 16:14

luca-s force-pushed the pyfolio_integration branch from 969235d to 3d1a6db Compare December 14, 2017 10:16

luca-s force-pushed the pyfolio_integration branch from 3d1a6db to e5bb76b Compare December 14, 2017 11:02

luca-s force-pushed the pyfolio_integration branch 3 times, most recently from 7b90c26 to 8469794 Compare December 14, 2017 12:50

luca-s force-pushed the pyfolio_integration branch from 48b6b50 to 2eb6372 Compare December 14, 2017 15:23

prsutherland reviewed Dec 14, 2017

View reviewed changes

luca-s force-pushed the pyfolio_integration branch 4 times, most recently from ff3f129 to 4784baa Compare December 19, 2017 09:32

Moved utils.cumulative_returns to performance.cumulative_returns

4121e6c

luca-s force-pushed the pyfolio_integration branch 2 times, most recently from 805c9f1 to 0518cb0 Compare December 19, 2017 20:55

luca-s added 4 commits December 19, 2017 22:15

Removed deprecated pd.TimeGrouper

a07429a

Added performance.create_pyfolio_input

00bc8e4

Added example NB for pyfolio integration feature

bf7c1a8

luca-s force-pushed the pyfolio_integration branch from 0518cb0 to bf7c1a8 Compare December 19, 2017 21:16

Updated "tear sheet walk through" NB

8ca5f6d

Make sure the NB reflect the new API

twiecki merged commit b2b4667 into quantopian:master Dec 19, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH Pyfolio integration #227

ENH Pyfolio integration #227

luca-s commented Dec 7, 2017 •

edited

luca-s commented Dec 7, 2017

luca-s commented Dec 7, 2017

luca-s commented Dec 8, 2017

twiecki Dec 8, 2017

twiecki commented Dec 8, 2017

luca-s commented Dec 8, 2017 •

edited

luca-s commented Dec 12, 2017

twiecki Dec 12, 2017

luca-s Dec 12, 2017

twiecki Dec 12, 2017

luca-s Dec 12, 2017 •

edited

twiecki Dec 12, 2017

twiecki commented Dec 12, 2017

luca-s commented Dec 12, 2017 •

edited

twiecki commented Dec 12, 2017

twiecki commented Dec 12, 2017

luca-s commented Dec 12, 2017 •

edited

abhijeetkalyan commented Dec 13, 2017

luca-s commented Dec 13, 2017

luca-s commented Dec 14, 2017

luca-s commented Dec 14, 2017

prsutherland left a comment

prsutherland Dec 13, 2017

luca-s Dec 14, 2017

prsutherland Dec 14, 2017

luca-s Dec 14, 2017

luca-s commented Dec 14, 2017 •

edited

luca-s commented Dec 19, 2017

twiecki commented Dec 19, 2017

		return columns[valid_columns]


		def timedelta_to_string(timedelta):



		def std_conversion(period_std):
		def std_conversion(period_std, one_period_len):

ENH Pyfolio integration #227

ENH Pyfolio integration #227

Conversation

luca-s commented Dec 7, 2017 • edited

luca-s commented Dec 7, 2017

luca-s commented Dec 7, 2017

luca-s commented Dec 8, 2017

Choose a reason for hiding this comment

twiecki commented Dec 8, 2017

luca-s commented Dec 8, 2017 • edited

luca-s commented Dec 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luca-s Dec 12, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twiecki commented Dec 12, 2017

luca-s commented Dec 12, 2017 • edited

twiecki commented Dec 12, 2017

twiecki commented Dec 12, 2017

luca-s commented Dec 12, 2017 • edited

abhijeetkalyan commented Dec 13, 2017

luca-s commented Dec 13, 2017

luca-s commented Dec 14, 2017

luca-s commented Dec 14, 2017

prsutherland left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luca-s commented Dec 14, 2017 • edited

luca-s commented Dec 19, 2017

twiecki commented Dec 19, 2017

luca-s commented Dec 7, 2017 •

edited

luca-s commented Dec 8, 2017 •

edited

luca-s Dec 12, 2017 •

edited

luca-s commented Dec 12, 2017 •

edited

luca-s commented Dec 12, 2017 •

edited

luca-s commented Dec 14, 2017 •

edited