FIX Avoid collisions when caching nested functions#1374
FIX Avoid collisions when caching nested functions#1374lesteve merged 10 commits intojoblib:masterfrom
Conversation
74cbb2d to
59cf40a
Compare
Codecov ReportBase: 93.94% // Head: 94.03% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #1374 +/- ##
==========================================
+ Coverage 93.94% 94.03% +0.09%
==========================================
Files 52 52
Lines 7328 7341 +13
==========================================
+ Hits 6884 6903 +19
+ Misses 444 438 -6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
the current implementation do not support nested function
if two nested function in different function have same name
such as this
```python
def func1():
def nested():
...
def func2():
def nested():
...
```
then the get_func_name will return same module and name pair
59cf40a to
54c1220
Compare
tomMoral
left a comment
There was a problem hiding this comment.
This looks like a good change. Would you mind adding a test case for this?
jeremiedbb
left a comment
There was a problem hiding this comment.
LGTM.
@tomMoral, @ogrisel, I added a test and a change log entry. Note that it generates a backward incompatible change since all cached nested functions will now have a different module name, which will invalid all such caches. We discussed that irl with @tomMoral and decided that there is no guarantee that the cache can be preserved between versions.
tomMoral
left a comment
There was a problem hiding this comment.
just a small comment but otherwise, LGTM!
joblib/test/test_func_inspect.py
Outdated
| cachedir = tmpdir_factory.mktemp("joblib_test_func_inspect") | ||
| mem = Memory(cachedir.strpath) | ||
|
|
||
| @mem.cache |
There was a problem hiding this comment.
do you need the memory here? If not, we can simplify
There was a problem hiding this comment.
Indeed we don't need any cached function. I simplified the test.
This test fails on master because the module are the same.
the current implementation do not support nested function
if two nested function in different function have same name
such as this
then the get_func_name will return same module and name pair