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

Fix drawdown behavior #297

Merged
merged 3 commits into from
May 11, 2016
Merged

Fix drawdown behavior #297

merged 3 commits into from
May 11, 2016

Conversation

brdsio
Copy link
Contributor

@brdsio brdsio commented May 10, 2016

Drawdown function does not work properly.
If one series had a drawdown from 2 to 1 and a drawdown from 10 to 6, pyfolio was considering the second one as the higher drawdown, but in fact the first drawdown was -50% and the second one was -40%.

This commit resolves this issue: #179

I added the tests for the both cases.

@twiecki
Copy link
Contributor

twiecki commented May 10, 2016

Thanks @flaviodrt! Still some PEP8 issues that should be easy to fix:

pyfolio/tests/test_timeseries.py:22:1: W293 blank line contains whitespace
pyfolio/tests/test_timeseries.py:24:1: W293 blank line contains whitespace
pyfolio/tests/test_timeseries.py:28:1: W293 blank line contains whitespace
pyfolio/tests/test_timeseries.py:29:5: E304 blank lines found after function decorator
pyfolio/tests/test_timeseries.py:35:49: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:36:49: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:40:1: W293 blank line contains whitespace
pyfolio/tests/test_timeseries.py:42:1: W293 blank line contains whitespace
pyfolio/tests/test_timeseries.py:56:22: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:57:56: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:58:57: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:59:58: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:62:1: W293 blank line contains whitespace
pyfolio/tests/test_timeseries.py:64:1: W293 blank line contains whitespace
pyfolio/tests/test_timeseries.py:66:1: W293 blank line contains whitespace
pyfolio/tests/test_timeseries.py:67:74: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:69:56: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:71:58: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:73:60: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:75:1: W293 blank line contains whitespace
pyfolio/tests/test_timeseries.py:76:74: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:78:56: W291 trailing whitespace
pyfolio/tests/test_timeseries.py:80:58: W291 trailing whitespace

(you can use flake8 to check and autopep8 to autofix)

@brdsio
Copy link
Contributor Author

brdsio commented May 10, 2016

Hi @twiecki!

Sorry for that.
I've sent a new commit with the fix.

@twiecki
Copy link
Contributor

twiecki commented May 10, 2016

Thanks. Some of the failures seem unrelated and are probably due to pandas 0.18.1. However, these two are related:

======================================================================
ERROR: test_gen_drawdown_table_relative_0 (pyfolio.tests.test_timeseries.TestDrawdown)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/nose_parameterized/parameterized.py", line 365, in standalone_func
    return func(*(a + p.args), **p.kwargs)
  File "/home/travis/build/quantopian/pyfolio/pyfolio/tests/test_timeseries.py", line 64, in test_gen_drawdown_table_relative
    drawdowns = timeseries.gen_drawdown_table(rets, top=2)
  File "/home/travis/build/quantopian/pyfolio/pyfolio/timeseries.py", line 1072, in gen_drawdown_table
    unit='D')
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/pandas/util/decorators.py", line 91, in wrapper
    return func(*args, **kwargs)
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/pandas/tseries/tools.py", line 291, in to_datetime
    unit=unit, infer_datetime_format=infer_datetime_format)
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/pandas/tseries/tools.py", line 420, in _to_datetime
    values = _convert_listlike(arg._values, False, format)
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/pandas/tseries/tools.py", line 331, in _convert_listlike
    errors=errors)
  File "pandas/tslib.pyx", line 1981, in pandas.tslib.array_with_unit_to_datetime (pandas/tslib.c:38273)
  File "pandas/tslib.pyx", line 2071, in pandas.tslib.array_with_unit_to_datetime (pandas/tslib.c:37194)
ValueError: non convertible value 2000-01-08with the unit 'D'
======================================================================
ERROR: test_get_max_drawdown_begins_first_day_0 (pyfolio.tests.test_timeseries.TestDrawdown)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/nose_parameterized/parameterized.py", line 365, in standalone_func
    return func(*(a + p.args), **p.kwargs)
  File "/home/travis/build/quantopian/pyfolio/pyfolio/tests/test_timeseries.py", line 30, in test_get_max_drawdown_begins_first_day
    drawdowns = timeseries.gen_drawdown_table(rets, top=1)
  File "/home/travis/build/quantopian/pyfolio/pyfolio/timeseries.py", line 1072, in gen_drawdown_table
    unit='D')
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/pandas/util/decorators.py", line 91, in wrapper
    return func(*args, **kwargs)
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/pandas/tseries/tools.py", line 291, in to_datetime
    unit=unit, infer_datetime_format=infer_datetime_format)
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/pandas/tseries/tools.py", line 420, in _to_datetime
    values = _convert_listlike(arg._values, False, format)
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/pandas/tseries/tools.py", line 331, in _convert_listlike
    errors=errors)
  File "pandas/tslib.pyx", line 1981, in pandas.tslib.array_with_unit_to_datetime (pandas/tslib.c:38273)
  File "pandas/tslib.pyx", line 2071, in pandas.tslib.array_with_unit_to_datetime (pandas/tslib.c:37194)
ValueError: non convertible value 2000-01-03with the unit 'D'

@brdsio
Copy link
Contributor Author

brdsio commented May 10, 2016

I think that were related to pandas 0.18.1 because the other tests in previous commits before my changes were failing too.
Removing the attribute unit from pd.to_datetime does not raise the exception.
Do you think that is a good solution?

@twiecki twiecki merged commit 5f5a9d5 into quantopian:master May 11, 2016
@twiecki
Copy link
Contributor

twiecki commented May 11, 2016

Yep, that looks great. Thanks for the fix!

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