Make it possible to pickle a fit GridSearchCV and RandomizedSearchCV
#1801
Conversation
…er search Avoids copying an unpicklable method to parameter search's __dict__. Also adds decision_function and transform where only predict and predict_proba were available before.
Define CVScoreTuple in the module namespace.
|
Great, I think this is the right solution. Not sure we need the underscore in front of the CVScoresTuple, though. Could you please make the test not only smoke tests? For example unpickle and see if the results of One downside of your solution is that it is no longer possible to ducktype the presence of |
|
I'll add some non-smoke testing of the pickle soon. I'm only smoke-testing To answer your second point (I hadn't been sure of the answer myself): class O(object):
@property
def a(self):
return self.bI.e. if This too should be tested somehow, but I'm not sure if it's for this patch, or generally an issue for meta-classifiers and functions only available after fit(). This suggests that anything relying on |
|
Actually it looks like And rethinking, I'm not really sure how/why to test pickling: isn't it enough to assume that if pickle works and is tested, as long as no error occurs it's fine? |
|
LGTM, 👍 for merge. (@GaelVaroquaux might like to give his opinion too, since this involves properties...) |
Valid usecase for properties: adaption pattern, which is what they are |
|
@amueller Merge? |
|
Sorry, have been offline for week as you might have noticed. LGTM. @jnothman If the runtime of a real test is basically the same as the smoke test, I don't see why we shouldn't do it. No idea what could break if you don't test for it ;) |
|
Merged by rebase. Thanks :) |
Fix for #1789:
best_estimator_'s methods available by copying its methods. Instead link to its attributes by properties (also, added properties fordecision_functionandtransformwherepredict,predict_probaandscorehad been available).CVScoresTuplein a closure.