Repository: joblib/joblib

Issue:
Surprising behaviour when one of the cached function arguments is not picklable
As reported by @arthurmensch. When one of the arguments is not picklable, the cached function result will only depend on the hash of all the arguments before the non picklable one.

A simple snippet to show the problem:
```python
import joblib

mem = joblib.Memory('/tmp/joblib')

@mem.cache()
def f(a, b):
    return b

non_picklable = lambda: None
print(f(non_picklable, 'first'))
print(f(non_picklable, 'second'))
```

Output:
```
/home/lesteve/dev/joblib/joblib/hashing.py:62: UserWarning: PicklingError while hashing {'b': 'first', 'a': <function <lambda> at 0x7f400d7ec8c8>}: PicklingError("Can't pickle <function <lambda> at 0x7f400d7ec8c8>: it's not found as __main__.<lambda>",)
  warnings.warn('PicklingError while hashing %r: %r' % (obj, e))
________________________________________________________________________________
[Memory] Calling __main__--tmp-test_hash_non_picklable_arguments.f...
f(<function <lambda> at 0x7f400d7ec8c8>, 'first')
________________________________________________________________f - 0.0s, 0.0min
first
/home/lesteve/dev/joblib/joblib/hashing.py:62: UserWarning: PicklingError while hashing {'b': 'second', 'a': <function <lambda> at 0x7f400d7ec8c8>}: PicklingError("Can't pickle <function <lambda> at 0x7f400d7ec8c8>: it's not found as __main__.<lambda>",)
  warnings.warn('PicklingError while hashing %r: %r' % (obj, e))
first
```

Why not just raise an exception in this case rather than returning a result with a warning that is almost certain to be ignored by the user ? @GaelVaroquaux @ogrisel.

Discussion:

Patch:
diff --git a/joblib/hashing.py b/joblib/hashing.py
index f8a9ee6..93bc5e3 100644
--- a/joblib/hashing.py
+++ b/joblib/hashing.py
@@ -59,7 +59,8 @@ class Hasher(Pickler):
         try:
             self.dump(obj)
         except pickle.PicklingError as e:
-            warnings.warn('PicklingError while hashing %r: %r' % (obj, e))
+            e.args += ('PicklingError while hashing %r: %r' % (obj, e),)
+            raise
         dumps = self.stream.getvalue()
         self._hash.update(dumps)
         if return_digest:


Test patch:
diff --git a/joblib/test/test_hashing.py b/joblib/test/test_hashing.py
index f0ce0eb..88407d0 100644
--- a/joblib/test/test_hashing.py
+++ b/joblib/test/test_hashing.py
@@ -23,6 +23,7 @@ from nose.tools import assert_equal
 from joblib.hashing import hash, PY3
 from joblib.func_inspect import filter_args
 from joblib.memory import Memory
+from joblib.testing import assert_raises_regex
 
 from joblib.test.test_memory import env as test_memory_env
 from joblib.test.test_memory import setup_module as test_memory_setup_func
@@ -429,3 +430,12 @@ def test_hashes_stay_the_same_with_numpy_objects():
 
     for to_hash, expected in zip(to_hash_list, expected_list):
         yield assert_equal, hash(to_hash), expected
+
+
+def test_hashing_pickling_error():
+    def non_picklable():
+        return 42
+
+    assert_raises_regex(pickle.PicklingError,
+                        'PicklingError while hashing',
+                        hash, non_picklable)
diff --git a/joblib/testing.py b/joblib/testing.py
index 8555a5a..e5cbae5 100644
--- a/joblib/testing.py
+++ b/joblib/testing.py
@@ -5,7 +5,7 @@ Helper for testing.
 import sys
 import warnings
 import os.path
-
+import re
 
 def warnings_to_stdout():
     """ Redirect all warnings to stdout.
@@ -17,3 +17,30 @@ def warnings_to_stdout():
 
     warnings.showwarning = showwarning
     #warnings.simplefilter('always')
+
+
+try:
+    from nose.tools import assert_raises_regex
+except ImportError:
+    # For Python 2.7
+    try:
+        from nose.tools import assert_raises_regexp as assert_raises_regex
+    except ImportError:
+        # for Python 2.6
+        def assert_raises_regex(expected_exception, expected_regexp,
+                                callable_obj=None, *args, **kwargs):
+            """Helper function to check for message patterns in exceptions"""
+
+            not_raised = False
+            try:
+                callable_obj(*args, **kwargs)
+                not_raised = True
+            except Exception as e:
+                error_message = str(e)
+                if not re.compile(expected_regexp).search(error_message):
+                    raise AssertionError("Error message should match pattern "
+                                         "%r. %r does not." %
+                                         (expected_regexp, error_message))
+            if not_raised:
+                raise AssertionError("Should have raised %r" %
+                                     expected_exception(expected_regexp))