[MRG+1] Added support for sample_weight in linearSVR, including tests and documentation. Fixes #6862 #6907
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| clf.fit(iris.data, iris.target) | ||
|
|
||
| prob_predict = clf.predict_proba(iris.data) | ||
| assert_array_almost_equal( | ||
| np.sum(prob_predict, 1), np.ones(iris.data.shape[0])) | ||
| assert_true(np.mean(np.argmax(prob_predict, 1) | ||
| == clf.predict(iris.data)) > 0.9) | ||
| == clf.predict(iris.data)) > 0.9) |
There was a problem hiding this comment.
Usually we wouldn't go fixing up cosmetic things when submitting an unrelated PR. It makes the PR somewhat harder to review. But at least this PR is small and focussed
|
Thanks for this. After a skim, it looks good, but I'll give it a closer look when I have time. |
|
|
||
| assert np.linalg.norm(lsvr.coef_ - lsvr_no_weight.coef_)\ | ||
| / np.linalg.norm(lsvr_no_weight.coef_) < .1 | ||
| assert np.abs(score1 - score2) < 0.1 |
There was a problem hiding this comment.
don't use assert but assert_less or assert_true
|
Frustratingly, you'll need to rebase on master. Otherwise looks good to me, and you should mention the enhancement in |
Replicate solution to scikit-learn@9a52077 except that `_pairwise` should always be `True` for `KernelCenterer` because it's supposed to receive a Gram matrix. This should make `KernelCenterer` usable in `Pipeline`s. Happy to add tests, just tell me what should be covered.
…g cython fused types (scikit-learn#6846)
This is a smoke test. Hence there is no point having cv=4
|
@jnothman I tried to rebase, not sure if I did the right thing. |
|
Rebase usually involves something like: |
|
I.e. it is not done correctly here |
c67fd55 to
65d1d93
Compare
|
Done! |
1 similar comment
|
Done! |
|
@agramfort, a quick review? |
|
thx @imaculate |
|
Pleasure! Thanks too for the guidance! |
olologin
pushed a commit
to olologin/scikit-learn
that referenced
this pull request
Aug 24, 2016
… and documentation. Fixes scikit-learn#6862 (scikit-learn#6907) * Make KernelCenterer a _pairwise operation Replicate solution to scikit-learn@9a52077 except that `_pairwise` should always be `True` for `KernelCenterer` because it's supposed to receive a Gram matrix. This should make `KernelCenterer` usable in `Pipeline`s. Happy to add tests, just tell me what should be covered. * Adding test for PR scikit-learn#6900 * Simplifying imports and test * updating changelog links on homepage (scikit-learn#6901) * first commit * changed binary average back to macro * changed binomialNB to multinomialNB * emphasis on "higher return values are better..." (scikit-learn#6909) * fix typo in comment of hierarchical clustering (scikit-learn#6912) * [MRG] Allows KMeans/MiniBatchKMeans to use float32 internally by using cython fused types (scikit-learn#6846) * Fix sklearn.base.clone for all scipy.sparse formats (scikit-learn#6910) * DOC If git is not installed, need to catch OSError Fixes scikit-learn#6860 * DOC add what's new for clone fix * fix a typo in ridge.py (scikit-learn#6917) * pep8 * TST: Speed up: cv=2 This is a smoke test. Hence there is no point having cv=4 * Added support for sample_weight in linearSVR, including tests and documentation * Changed assert to assert_allclose and assert_almost_equal, reduced the test tolerance * Fixed pep8 violations and sampleweight format * rebased with upstream
TomDLT
pushed a commit
to TomDLT/scikit-learn
that referenced
this pull request
Oct 3, 2016
… and documentation. Fixes scikit-learn#6862 (scikit-learn#6907) * Make KernelCenterer a _pairwise operation Replicate solution to scikit-learn@9a52077 except that `_pairwise` should always be `True` for `KernelCenterer` because it's supposed to receive a Gram matrix. This should make `KernelCenterer` usable in `Pipeline`s. Happy to add tests, just tell me what should be covered. * Adding test for PR scikit-learn#6900 * Simplifying imports and test * updating changelog links on homepage (scikit-learn#6901) * first commit * changed binary average back to macro * changed binomialNB to multinomialNB * emphasis on "higher return values are better..." (scikit-learn#6909) * fix typo in comment of hierarchical clustering (scikit-learn#6912) * [MRG] Allows KMeans/MiniBatchKMeans to use float32 internally by using cython fused types (scikit-learn#6846) * Fix sklearn.base.clone for all scipy.sparse formats (scikit-learn#6910) * DOC If git is not installed, need to catch OSError Fixes scikit-learn#6860 * DOC add what's new for clone fix * fix a typo in ridge.py (scikit-learn#6917) * pep8 * TST: Speed up: cv=2 This is a smoke test. Hence there is no point having cv=4 * Added support for sample_weight in linearSVR, including tests and documentation * Changed assert to assert_allclose and assert_almost_equal, reduced the test tolerance * Fixed pep8 violations and sampleweight format * rebased with upstream
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issue
What does this implement/fix? Explain your changes.
Any other comments?
…umentation