FIX Avoid collisions when caching nested functions#1374
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
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
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
tomMoral
left a comment
There was a problem hiding this comment.
just a small comment but otherwise, LGTM!
| 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.
lesteve
changed the title
4 participants
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