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 deprecations and remove Bayesian models #608

Merged
merged 51 commits into from Sep 20, 2019
Merged

Fix deprecations and remove Bayesian models #608

merged 51 commits into from Sep 20, 2019

Conversation

twiecki
Copy link
Contributor

@twiecki twiecki commented Aug 15, 2019

This fixes a whole bunch of stuff that caused tests to break under more recent python and pandas. It also removes the bayesian module, it's just making tests slow, is difficult to install, and we don't have time to maintain it properly.

@twiecki
Copy link
Contributor Author

twiecki commented Sep 4, 2019

@huaiweicheng Let me know if you need help.

@huaiweicheng
Copy link
Contributor

@twiecki Some other work cost me some time. Will continue in near days.

@@ -39,7 +39,7 @@ class RoundTripTestCase(TestCase):
],
columns=['amount', 'price', 'symbol'],
index=dates_intraday[[0, 2]])
.rename_axis('dt', axis='index')
.rename('dt', axis='index')
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be wrong. revert it.

@@ -53,7 +53,7 @@ class RoundTripTestCase(TestCase):
],
columns=['amount', 'price', 'symbol'],
index=dates_intraday[[0, 4]])
.rename_axis('dt', axis='index')
.rename('dt', axis='index')
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be wrong. revert it.

@huaiweicheng
Copy link
Contributor

@twiecki Reverting the above codes and I have passed all the test right now. Did you have any suggestions? (fyi, never used round_trip before and not familiar with that part)

@twiecki
Copy link
Contributor Author

twiecki commented Sep 6, 2019

@huaiweicheng Want to do another PR with that fix?

@huaiweicheng
Copy link
Contributor

@twiecki Sorry to get you late. Of course I'll do.

@huaiweicheng
Copy link
Contributor

@twiecki python3.7 works. python2.7 did not pass. I notice the failed reason is that the difference of nan & 0. Could you help locating that?

@huaiweicheng
Copy link
Contributor

@twiecki I find another problem. The

    for column in perf_stats.columns:
        for stat, value in perf_stats[column].iteritems():
            if stat in STAT_FUNCS_PCT:
                perf_stats.loc[stat, column] = str(np.round(value * 100,
                                                            1)) + '%

code in line 650 of plotting.py makes the precision to 1 which is to small. My suggestion is that precision should be at least 2 or 3.

@twiecki twiecki merged commit c5ebc00 into master Sep 20, 2019
@twiecki
Copy link
Contributor Author

twiecki commented Sep 20, 2019

We did it @huaiweicheng!! Thanks so much for your help.

@twiecki
Copy link
Contributor Author

twiecki commented Sep 20, 2019

@richafrank Probably should have looped you in before merging, but this fixes a bunch of stuff that breaks under new pandas (but keeps it running on 0.18) and removes a bunch of stuff which we don't need anymore. Mainly the Bayes tearsheet and the risk tearsheet (superseded by the perf_attrib tearsheet).

@twiecki
Copy link
Contributor Author

twiecki commented Sep 20, 2019 via email

@huaiweicheng
Copy link
Contributor

Hi, twiecki. How about adding turnover_denom para in create_txn_tear_sheet function? turnover_denom is one para in create_returns_tear_sheet but not in create_txn_tear_sheet which is weird. I could do the PR.

@twiecki
Copy link
Contributor Author

twiecki commented Sep 27, 2019 via email

@huaiweicheng
Copy link
Contributor

@twiecki Yeah. I use the create_returns_tear_sheet to produce a brief summary data of returns, ratios, turnovers and so on. The turnover_denom param I set it to 'portfolio_value'. Meanwhile I use create_txn_tear_sheet to generate the report, I find it has no turnover_denom param. Thus, the turnover data generated by create_returns_tear_sheet and the plot generated by create_txn_tear_sheet does not match. I open a new PR #618, which locate the problem. And also, it fix some small issues such as useless params.

@steelep
Copy link

steelep commented Jul 5, 2020

This fixes a whole bunch of stuff that caused tests to break under more recent python and pandas. It also removes the bayesian module, it's just making tests slow, is difficult to install, and we don't have time to maintain it properly.

If I spent some time to reincorporate the bayesian module - would you be open to this? It was super useful.

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.

None yet

3 participants