[MRG+1] FIX Make sure GridSearchCV and RandomizedSearchCV are pickle-able #7594
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
|
[The flake8 test will fail because of the utils.fixes import] |
If you really care you can do something like this to silence the warning: Or we can just merge like this obviously. |
lesteve
approved these changes
Oct 7, 2016
|
|
||
| if np_version < (1, 12, 0): | ||
| class MaskedArray(np.ma.MaskedArray): | ||
| # Before numpy 1.12, np.ma.MaskedArray object is not pickle-able |
There was a problem hiding this comment.
I would replace pickle-able by picklable which is an acceptable adjective I think (it is definitely already mentioned a few times in the source code).
Ah... Cool. I recall seeing this somewhere! |
Thanks for the review @lesteve!! |
amueller
requested changes
Oct 7, 2016
|
|
||
| random_search = RandomizedSearchCV(clf, {'foo_param': [1, 2, 3]}, | ||
| refit=True, n_iter=3) | ||
| random_search.fit(X, y) | ||
| pickle.dumps(random_search) # smoke test | ||
| pickle.loads(pickle.dumps(random_search)) |
| @@ -940,12 +940,12 @@ def test_pickle(): | |||
| clf = MockClassifier() | |||
| grid_search = GridSearchCV(clf, {'foo_param': [1, 2, 3]}, refit=True) | |||
| grid_search.fit(X, y) | |||
| pickle.dumps(grid_search) # smoke test | |||
| pickle.loads(pickle.dumps(grid_search)) | |||
5596417 to
82e4d0a
Compare
|
@amueller Thanks for the review. Have rebased and addressed your comment... The meta tests don't seem to test pickle :/ I'll confirm tomorrow... |
amueller
approved these changes
Oct 10, 2016
|
LGTM |
amueller
added a commit
to amueller/scikit-learn
that referenced
this pull request
Oct 14, 2016
…able (scikit-learn#7594) * FIX Subclass a new MaskedArray which allows pickling even when dype=object * TST unpickling too * FIX Use MaskedArray from utils.fixes rather than from numpy * FIX imports * Don't assign a variable * FIX np --> numpy * Use tostring instead of tobytes for old numpy * COSMIT pickle-able --> picklable * use #noqa comment to turn off flake8 * TST/ENH Check if the pickled est's predict matches with the original one's # Conflicts: # sklearn/utils/tests/test_fixes.py
|
needs a whatsnew :-/ |
Sundrique
pushed a commit
to Sundrique/scikit-learn
that referenced
this pull request
Jun 14, 2017
…able (scikit-learn#7594) * FIX Subclass a new MaskedArray which allows pickling even when dype=object * TST unpickling too * FIX Use MaskedArray from utils.fixes rather than from numpy * FIX imports * Don't assign a variable * FIX np --> numpy * Use tostring instead of tobytes for old numpy * COSMIT pickle-able --> picklable * use #noqa comment to turn off flake8 * TST/ENH Check if the pickled est's predict matches with the original one's
paulha
pushed a commit
to paulha/scikit-learn
that referenced
this pull request
Aug 19, 2017
…able (scikit-learn#7594) * FIX Subclass a new MaskedArray which allows pickling even when dype=object * TST unpickling too * FIX Use MaskedArray from utils.fixes rather than from numpy * FIX imports * Don't assign a variable * FIX np --> numpy * Use tostring instead of tobytes for old numpy * COSMIT pickle-able --> picklable * use #noqa comment to turn off flake8 * TST/ENH Check if the pickled est's predict matches with the original one's
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.
Fixes #7562
np.ma.MaskedArrayand overrides the__getstate__to make obj dtypedMaskedArrays pickle-able.utils.fixes.MaskedArrayinsidegs.cv_results_...This is based off of numpy/numpy#8122
Please review @jnothman @amueller @GaelVaroquaux @davechallis