zipline-live / zipline

Zipline-Live, a Pythonic Algorithmic Trading Library

http://www.zipline-live.io/

Apache License 2.0

395 stars 65 forks source link

live is broken? #16

Closed bartosh closed 7 years ago

bartosh commented 7 years ago

Hi,

PR 15 fails tests with below error. I suspect that this is caused by changes in 'live'. How come they've got merged if they're failing tests?

====================================================================== ERROR: test_panel_data_0_daily (tests.test_algorithm.TestPanelData)

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/zipline-live/zipline/tests/test_algorithm.py", line 4750, in test_panel_data data=panel File "/home/travis/build/zipline-live/zipline/zipline/utils/run_algo.py", line 384, in run_algorithm environ=environ, TypeError: _run() takes exactly 20 arguments (18 given)

Regards, Ed

tibkiss commented 7 years ago

Thanks for reporting, I have a fix for these issues. I'm just waiting for https://github.com/zipline-live/zipline/pull/12 to get merged as the fix will likely to be conflicting with that PR.

bartosh commented 7 years ago

12 needs to be rebased on top of benchmark fix, I'm afraid. I think it's better to merge the fix, then fix tests in live branch. After that we can get back to this PR.

tibkiss commented 7 years ago

Create a PR which fixes the testcases: https://github.com/zipline-live/zipline/pull/17

bartosh commented 7 years ago

I thought it's not possible to fix travis and AppVeyor builds without merging PR #15. Are you going to merge it anytime soon?

tibkiss commented 7 years ago

I misunderstood your comment: thought you are suggesting that we shall merge the testcase fix first. Re-reading it clarified the intent.

I cannot merge #12 as it is in conflict state. Please resolve the conflict then I'll merge it ASAP.

bartosh commented 7 years ago

Sorry for this confusion. Correct order of merging is: #15, #17, #12. I'll rebase #12 as soon as both tests suites are 'green'.

tibkiss commented 7 years ago

Thanks for the clarification. Merged #15, will fix #17 soon, then we can proceed with #12.

tibkiss commented 7 years ago

I believe this could be closed now.