Add support for weakref.ref type

[mrocklin]

Tornado 4.5 is using this in all coroutines.

I don't see a clear way to serialize it. I've placed it in a global
dictionary of singletons. I would be delighted to find another way
to do this.

[mrocklin]

Hrm, still getting a problem on tornado coroutines that is a bit confusing:

In [1]: from tornado import gen
   ...: 
   ...: @gen.coroutine
   ...: def f():
   ...:     pass
   ...:     
   ...: from cloudpickle import dumps, loads
   ...: loads(dumps(f))
   ...: 
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-432aac7176dc> in <module>()
      6 
      7 from cloudpickle import dumps, loads
----> 8 loads(dumps(f))

TypeError: __new__ expected at least 1 arguments, got 0

[codecov-io]

Current coverage is 78.13% (diff: 87.50%)

Merging #67 into master will increase coverage by 0.16%

@@             master        #67   diff @@
==========================================
  Files             2          2          
  Lines           463        471     +8   
  Methods           0          0          
  Messages          0          0          
  Branches         91         94     +3   
==========================================
+ Hits            361        368     +7   
- Misses           73         74     +1   
  Partials         29         29          

Powered by Codecov. Last update 0d225a4...a256251

[rgbkrk]

/cc @minrk

[mrocklin]

Also cc @pitrou, in case he has thoughts on how this might be solved more effectively.

[pitrou]

Hmm, I'm not sure I understand. Do Tornado coroutines have a reference to weak reference objects or only to the weakref.ref type? If the latter, it should be easily serializable as a global.

[mrocklin]

When I track the exception it seems to fail on the type. I suspect that this is because of the following:

In [1]: from weakref import ref

In [2]: type(ref).__module__
Out[2]: 'builtins'

[pitrou]

The introduction of weak references seems to have been made in changeset 24a8b22 (tornadoweb/tornado#1782). We may have to teach cloudpickle to serialize WeakKeyDictionary objects...

[pitrou]

Now that I'm thinking about it again, serializing WeakKeyDictionary wouldn't help, because we want Futures to be stored in that particular WeakKeyDictionary instance (the global one in tornado.gen), not recreate another one when deserializing.

So we would need specific cloudpickle support for Tornado coroutines.

[pitrou]

Looking at cloudpickle's architecture, it sounds like specific support for Tornado coroutines would have to go in Dask after subclassing the Pickler class. @rgbkrk, does that sound right?

(note: there is no direct way to typecheck a Tornado coroutine, as it's a regular nested function. One has to inspect the __code__ attribute)

[rgbkrk]

Yeah, in order to serialize custom objects from other libraries it either has to be added directly here (if widespread enough) or subclassed in the library using it. It would be nice if the support was in tornado directly, for the benefit of IPython parallel I assume. However, I don't have a vested opinion here as I'm not directly using this library (only indirectly through dask, pyspark, ipyparallel).

[pitrou]

It would be nice if the support was in tornado directly, for the benefit of IPython parallel I assume.

I'm not sure sure how to do that. To be clear, we need to change the code in Pickler or to subclass it...

[pitrou]

Implemented at dask/distributed#673.

[rgbkrk]

Given #69 has been merged, will you be revisiting this @mrocklin?