[MRG] Fix sklearn.base.clone when estimator has any kind of sparse matrix as attribute #6910
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
9681190 to
147055f
Compare
|
The test fails with Python 2.6. |
| data = arr.data if sparse.issparse(arr) else arr | ||
| return data.flat[0], data.flat[-1] | ||
| except AttributeError: | ||
| # Sparse matrices without .data attribute. Only dok_matrix at |
|
LGTM when test failure sorted. |
a98038a to
a56522c
Compare
|
|
||
| PY26 = sys.version_info[:2] == (2, 6) | ||
| if PY26: | ||
| # sp.dok_matrix can not be deepcopied in Python 2.6 |
There was a problem hiding this comment.
For the record:
from copy import deepcopy
import numpy as np
from scipy import sparse as sp
m = sp.dok_matrix(np.eye(5))
deepcopy(m)fails on Python 2.6 with the error:
AttributeError: shape not found
For some reason it looks like the reconstructed matrix is missing some attributes. I think it's fine not testing dok_matrix for Python 2.6.
There was a problem hiding this comment.
Yes, I think it's because Py2.6 has a specialised reconstruction routine for dicts and fails to handle the subclassing correctly... I'm happy with this solution.
when estimator has any kind of sparse matrix as attribute. Add test.
a56522c to
808d584
Compare
|
Thanks @lesteve. I'll throw in a what's new. |
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
Fix #6855.