Add memory to make_pipeline function #458
Conversation
chkoar
changed the title
|
Just put an entry in what's new as a bug fix. Good catch |
|
Oh and a regression test |
|
Hello @chkoar! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on August 26, 2018 at 11:15 Hours UTC |
Codecov Report
@@ Coverage Diff @@
## master #458 +/- ##
=========================================
Coverage ? 98.78%
=========================================
Files ? 75
Lines ? 4628
Branches ? 0
=========================================
Hits ? 4572
Misses ? 56
Partials ? 0
Continue to review full report at Codecov.
|
imblearn/tests/test_pipeline.py
Outdated
| pipeline = make_pipeline(DummyTransf(), SVC(), memory=memory) | ||
| assert_true(pipeline.memory is memory) | ||
| pipeline = make_pipeline(DummyTransf(), SVC()) | ||
| assert_true(pipeline.memory is None) |
There was a problem hiding this comment.
Use assert pipeline.memory is None
imblearn/tests/test_pipeline.py
Outdated
| cachedir = mkdtemp() | ||
| if LooseVersion(joblib_version) < LooseVersion('0.12'): | ||
| # Deal with change of API in joblib | ||
| memory = Memory(cachedir=cachedir, verbose=10) |
There was a problem hiding this comment.
I would not bother. I would make the one that work and update when the test will break when we will update the scikit learn version
imblearn/tests/test_pipeline.py
Outdated
| else: | ||
| memory = Memory(location=cachedir, verbose=10) | ||
| pipeline = make_pipeline(DummyTransf(), SVC(), memory=memory) | ||
| assert_true(pipeline.memory is memory) |
imblearn/tests/test_pipeline.py
Outdated
| pipeline = make_pipeline(DummyTransf(), SVC()) | ||
| assert_true(pipeline.memory is None) | ||
|
|
||
| shutil.rmtree(cachedir) |
There was a problem hiding this comment.
You need a try ... finally to be sure to remove the folder even if the test fail
|
@glemaitre does this considered as a bug fix or as an enhancement? |
|
Bugs I would say. It should have been there. |
|
@glemaitre ok |
|
Tanks |
What does this implement/fix? Explain your changes.
Adds the ability to pass a joblib.Memory object to the
make_pipelinefunction.