-
Notifications
You must be signed in to change notification settings - Fork 434
FIX: FileSystemStoreBackend string representation only returning location (path) #765
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
@@ Coverage Diff @@
## master #765 +/- ##
==========================================
- Coverage 95.42% 95.35% -0.07%
==========================================
Files 44 44
Lines 6224 6245 +21
==========================================
+ Hits 5939 5955 +16
- Misses 285 290 +5
Continue to review full report at Codecov.
|
Can you check that self.store_backend is not used in other classes |
I just did and you are right it was used in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a small comment to simplify a test.
joblib/test/test_memory.py
Outdated
def test_filesystem_store_backend_repr(tmpdir): | ||
# Verify string representation of a filesystem store backend. | ||
|
||
repr_pattern = '{class_name}(location="{location}")' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class_name
is always FileSystemStoreBackend
in this test. Please just write:
repr_pattern = 'FileSystemStoreBackend(location="{location}")'
and then simplify the test accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed! I just pushed the fix.
@ogrisel, is it ok to merge ? |
joblib/test/test_memory.py
Outdated
memory = Memory(location=tmpdir.strpath, verbose=0) | ||
memorized_func = memory.cache(my_func) | ||
|
||
memorized_func_repr = '{class_name}(func={func}, location={location})' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually class_name
is always "MemorizedFunc"
for memorized_func_repr
and always "MemorizedResult"
for memorized_result_repr
so this test can also be simplified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed... Also done with Memory.
* tag '0.12.3': (23 commits) Release 0.12.3 Loky 2.2.1 (joblib#760) FIX: FileSystemStoreBackend string representation only returning location (path) (joblib#765) Add optional dependency on psutil MAINT remove brittle time based assertion in test (joblib#761) Fix a bug in nesting level computation with FallbackToBackend(SequentialBackend()) (joblib#759) Make docstring more consistent with project style Improved performance of call_and_shelve (joblib#757) Better test name Fix default context handling (joblib#754) cloudpickle 0.5.5 (joblib#756) Fixed error where filter args was consuming kwargs (joblib#750) FIX pickle roundtrip for Memory and related classes. (joblib#746) test that passes but timeout appveyor because of unclosed semaphore tracker (joblib#676) DOC: fine tune compressor example dataset size for ReadTheDocs (joblib#753) FIX: MemorizedResult not picklable (joblib#752) [MRG] Better message with py27 when lz4 is not installed (joblib#740) MNT remove mutable default value for backend_options parameter (joblib#748) MNT create test file for _store_backends module (joblib#749) DOC consistently use memory rather than mem in memory.rst (joblib#744) ...
* releases: (23 commits) Release 0.12.3 Loky 2.2.1 (joblib#760) FIX: FileSystemStoreBackend string representation only returning location (path) (joblib#765) Add optional dependency on psutil MAINT remove brittle time based assertion in test (joblib#761) Fix a bug in nesting level computation with FallbackToBackend(SequentialBackend()) (joblib#759) Make docstring more consistent with project style Improved performance of call_and_shelve (joblib#757) Better test name Fix default context handling (joblib#754) cloudpickle 0.5.5 (joblib#756) Fixed error where filter args was consuming kwargs (joblib#750) FIX pickle roundtrip for Memory and related classes. (joblib#746) test that passes but timeout appveyor because of unclosed semaphore tracker (joblib#676) DOC: fine tune compressor example dataset size for ReadTheDocs (joblib#753) FIX: MemorizedResult not picklable (joblib#752) [MRG] Better message with py27 when lz4 is not installed (joblib#740) MNT remove mutable default value for backend_options parameter (joblib#748) MNT create test file for _store_backends module (joblib#749) DOC consistently use memory rather than mem in memory.rst (joblib#744) ...
Fix #764
A non regression test is also provided.
With this PR, the snippets provided in #764 not returns the following: