Added the option to defer updating when adding stocks for use in buil…

[nuvious]

…ding a portfolio. This improve performance of that funciton by reducing the number of calls to pf._update() to 1 instead of n for n number of stocks.

[nuvious]

Tried linking the PR to the related issue but don't seem to have that option.

[nuvious]

@fmilthaler, just a prod about this particular PR.

[fmilthaler]

Hey, thanks for adding this. I'll have a closer look at this change over the weekend. Being quite busy at work these days. :/

[fmilthaler]

With those changes, the following tests are failing:

• test_portfolio.py::test_maximum_sharpe_ratio FAILED
• test_portfolio.py::test_efficient_return FAILED
• test_portfolio.py::test_efficient_volatility FAILED
• test_portfolio.py::test_efficient_frontier FAILED
• test_portfolio.py::test_plot_efrontier FAILED
• test_portfolio.py::test_plot_optimal_portfolios FAILED
• test_portfolio.py::test_plot_stocks FAILED

[fmilthaler]

Since you noticed an impact on performance, how many stocks do you have in your portfolio?

[fmilthaler]

@nuvious can you look into the above tests failing?

[nuvious]

I did a quick check on my end and am seeing the same failures. Question regarding the pf._update() function, is it supposed to respond differently if you call it after every stock vs if you add multiple stocks at once and then call it? To answer your question I'm working with 100's of stocks at a time in the portfolio stuff I'm fiddling with in my spare time.

[fmilthaler]

I think I designed it to mainly recalculate/update portfolio properties every time a new stock was added. Initially I aimed to provide functions to the user to add stocks post creating a portfolio. This is actually something I wanted to enable some time ago, and I do have a branch that would do exactly that, but needs a bit of attention before it is merged into master. Alongside that change, pf._update()'s logic/implementation would also change. If you find an implementation which passes all tests and allows for deferring the update if multiple stocks are added, I'd be happy. I guess your branch is almost there, just need to figure out where the tests go wrong.

[nuvious]

So I did some more digging today and ran the test, but they all error out on Quandl api issues. I've set my ~/.quandl_apikey file and even edited the quandl package to verify it was grabbing the key. I couldn't see an optional parameter to set the api key explicitly in FinQuant, but without it both the master branch and my fork fail all the tests you highlighted. Any advice?

[fmilthaler]

Sorry for the late reply. You can set your quandl api key as a environment variable QUANDLAPIKEY=<your quandl api key>

[fmilthaler]

Or, since I got everything set up for testing purposes on Travis, what you could do is make changes in the code, commit, push to your fork, and once you want it tested, I could merge it into a branch within FinQuant, so that Travis runs all the tests. This would of course delay things a bit more, as you won't get the test results as soon as you are ready.

See if you can set your quandl api key as an environment variable. Then you could run the tests successfully locally, which would make it easier.

• If you are on windows, try this: https://support.shotgunsoftware.com/hc/en-us/articles/114094235653-Setting-global-environment-variables-on-Windows
• If you are on Linux, simply add the following to your ~/.bashrc export QUANDLAPIKEY=<your api key>

Hope this help, and once again, thanks for the help! :)

[nuvious]

Going to close this because it's stale just to clean up my github a bit. Still interested in this project but my issue may have been on my end and my own project has mothballed since initial implementaiton.

[fmilthaler]

Hey @nuvious I know it's been like forever, but FinQuant was recently resurrected. I'd like to merge this straight into master. Obviously I could recreate the changes you made here, but I'd like you to get the credit for it and be listed as a contributor. Would you mind creating a new fork/branch and PR?

This time, the tests are going to work, no more issues with quandl, you don't even need a key yourself, it's all handled on FinQuant's side now (besides, quandl is dead and it's support will be removed).

[nuvious]

Hey, belated response but glad to see that code was useful! Let me know if you need any refinement to all that.

[fmilthaler]

Certainly was useful, just that I ignore the project for several years ;) I just wanted to ask you, if you wanted to put up another PR for this, so that you get the full credit (being a contributor with a commit in master) for the change when it is merged. If you don't care about it, I'm happy to make the change myself.

[nuvious]

@fmilthaler, re-forked and submitted. Ran unit tests under a docker image and everyhting looked good. Will watch the CICD and/or standing by for any direction on the change. Thanks again for providing the opportunity to formally contribute as well!