search

fmilthaler / FinQuant

A program for financial portfolio management, analysis and optimisation.
MIT License
1.38k stars 192 forks source link

minor bug fix (.iloc[i] instead of .loc[i]) and optimization on portfolio #32

Closed fmilthaler closed 1 year ago

fmilthaler commented 5 years ago

Using .iloc[i] instead of .loc[i], and calling pf._update only once, after portfolio was generated. This closes #31

PietropaoloFrisoni commented 1 year ago

It seems indeed much more efficient to update all quantities only at the end of building the portfolio (I also wanted to do something similar in the first place). I assumed updating every time was a deliberate design choice so that users could add/remove stocks and funds dynamically by automatically updating the portfolio. Tests seem to work, so I approved the changes. I hope no strange and unexpected behaviors will appear.

fmilthaler commented 1 year ago

Thanks @PietropaoloFrisoni for the review. I ran through all examples and all looks good there as well. I think that I originally designed it in a way for the user to manually add a stock to the portfolio post its creation. But that feature is not in master, nor do I think there is much need for it. Hence, the update after all stocks were added makes sense.

fmilthaler commented 1 year ago

A similar solution regarding the _update calls is in the closed PR #58. Contacted the author to try and recreated their PR as their solution also adds an argument defer_update.