-
Notifications
You must be signed in to change notification settings - Fork 855
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
Stable ols #575
Stable ols #575
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Looks great so far. Here are some additional suggestions
Thanks for adding the descriptions to the notebook! I refined these a little bit, please have a look if they sound good to you. Btw., I removed a bunch of these references, because I didn't know where they should be cited (there were no citation indicators in the notebook before). Currently, I only cite ELS for QR factorization and Strang for the SVD approach. However, if the other citations you had fit for additional sections, please add back the references with citation numbers where appropriate. Otherwise, if this looks good to you, I'd be happy to merge this once the unit tests pass. Thanks! |
Looks Great!
Should we add something on matrices being ill-conditioned? Like this: |
Sure, good point, we could add a reference via the link you cited. I.e.,
ReferencesI believe this will be very useful to many users. |
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for finding and adding this reference. I have some suggestions with regard to adding it. See below. Thanks!
Thanks for the changes and the whole PR! Should be good to merge now. |
Thanks @rasbt for being so cooperative and patient! |
Description
Added new features to OLS function. Now you can also use SVD and QR decompostion
methods to for Linear Regression.
Related issues or pull requests
It is related to issue #56, 'Numerically Stable OLS'.
Applies #56
Pull Request Checklist
./docs/sources/CHANGELOG.md
file (if applicable)./mlxtend/*/tests
directories (if applicable)mlxtend/docs/sources/
(if applicable)PYTHONPATH='.' pytest ./mlxtend -sv
and make sure that all unit tests pass (for small modifications, it might be sufficient to only run the specific test file, e.g.,PYTHONPATH='.' pytest ./mlxtend/classifier/tests/test_stacking_cv_classifier.py -sv
)flake8 ./mlxtend