[MRG+1] Make KernelCenterer a _pairwise operation #6900
Merged
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
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.
|
can you add a test? |
| # NB: this test is pretty vacuous -- it's mainly to test integration | ||
| # of Pipeline and KernelCenterer | ||
| y_pred = cross_val_predict(pipeline,K,y_true,cv=4) | ||
| assert_array_almost_equal(y_true, y_pred) |
There was a problem hiding this comment.
please run a pep8 checker on this code and ping us when done
|
Done, I only fixed the stuff that came up in my changes |
|
LGTM +1 for merge |
| # test cross-validation, score should be almost perfect | ||
| # NB: this test is pretty vacuous -- it's mainly to test integration | ||
| # of Pipeline and KernelCenterer | ||
| y_pred = cross_val_predict(pipeline, K, y_true, cv=4) |
There was a problem hiding this comment.
Could you put cv=2, to reduce computing power.
There was a problem hiding this comment.
Forget it. I'll merge and do it.
imaculate
pushed a commit
to imaculate/scikit-learn
that referenced
this pull request
Jun 23, 2016
agramfort
pushed a commit
that referenced
this pull request
Jun 23, 2016
… and documentation. Fixes #6862 (#6907) * Make KernelCenterer a _pairwise operation Replicate solution to 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 #6900 * Simplifying imports and test * updating changelog links on homepage (#6901) * first commit * changed binary average back to macro * changed binomialNB to multinomialNB * emphasis on "higher return values are better..." (#6909) * fix typo in comment of hierarchical clustering (#6912) * [MRG] Allows KMeans/MiniBatchKMeans to use float32 internally by using cython fused types (#6846) * Fix sklearn.base.clone for all scipy.sparse formats (#6910) * DOC If git is not installed, need to catch OSError Fixes #6860 * DOC add what's new for clone fix * fix a typo in ridge.py (#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
olologin
pushed a commit
to olologin/scikit-learn
that referenced
this pull request
Aug 24, 2016
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
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
#803 is the relevant PR for fixing
KernelPCAinPipelinesWhat does this implement/fix? Explain your changes.
Allows for using
KernelCentererinPipelines. EDIT: More specifically, inPipelines that you want to run CV on.Any other comments?
Replicate solution to 9a52077 except that
_pairwiseshould always beTrueforKernelCentererbecause it's supposed to receive a Gram matrix. This should makeKernelCentererusable inPipelines.Happy to add tests, just tell me what should be covered.