Skip to content
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

dynamically generated class methods fail to pickle #58

Open
mattja opened this issue Jul 22, 2014 · 8 comments
Open

dynamically generated class methods fail to pickle #58

mattja opened this issue Jul 22, 2014 · 8 comments

Comments

@mattja
Copy link

mattja commented Jul 22, 2014

When using full class pickling (e.g. cls.__module__ = '__main__'), pickling
dynamically generated class methods fails with maximum recursion depth exceeded
Looks like a separate issue from #56. Maybe due to handling of the circular references?
Class.__dict__ --> method ; method.im_class --> Class

test case:

import dill
import types

def proto_method(self):
    pass

def make_class(name):
    cls = type(name, (object,), dict())
    setattr(cls, 'methodname', types.MethodType(proto_method, None, cls))
    globals()[name] = cls
    return cls

if __name__ == '__main__':
    NewCls = make_class('NewCls')
    print(dill.pickles(NewCls))

dill.detect.trace:

INFO:dill:T2: <class '__main__.NewCls'>
INFO:dill:F2: <function _create_type at 0x7fe706a281b8>
INFO:dill:T1: <type 'type'>
INFO:dill:F2: <function _load_type at 0x7fe706a28140>
INFO:dill:T1: <type 'object'>
INFO:dill:D2: <dict object at 0x7fe706a31e88>
INFO:dill:Me: <unbound method NewCls.proto_method>
INFO:dill:T1: <type 'instancemethod'>
INFO:dill:F1: <function proto_method at 0x7fe70f2a1a28>
INFO:dill:F2: <function _create_function at 0x7fe706a28230>
INFO:dill:Co: <code object proto_method at 0x7fe710832530, file "/path/test.py", line 4>
INFO:dill:F2: <function _unmarshal at 0x7fe706a280c8>
INFO:dill:D4: <dict object at 0x7fe706a32280>
INFO:dill:D2: <dict object at 0x7fe706a1ab40>
INFO:dill:T2: <class '__main__.NewCls'>
INFO:dill:D2: <dict object at 0x7fe706a22c58>
INFO:dill:Me: <unbound method NewCls.proto_method>
INFO:dill:T2: <class '__main__.NewCls'>
INFO:dill:D2: <dict object at 0x7fe706a20b40>
INFO:dill:Me: <unbound method NewCls.proto_method>
INFO:dill:T2: <class '__main__.NewCls'>
INFO:dill:D2: <dict object at 0x7fe706a22d70>
INFO:dill:Me: <unbound method NewCls.proto_method>
...etc...
@matsjoyce
Copy link
Contributor

Yup, its the circular dependency I think I've highlighted:

newcls

I don't know why it differs so much from a "normal" class:

newcls2

@mattja
Copy link
Author

mattja commented Jul 22, 2014

In the test above if I change line 9 to setattr(cls, 'methodname', proto_method) instead of explicitly using MethodType then the test passes.

But when I try that approach with the real program, dill still fails on circular references (but with a cell involved)

INFO:dill:Co: <code object method at 0x7f9be4e08230, file "/path/to.py", line 625>
INFO:dill:F2: <function _unmarshal at 0x7f9bed9d2140>
INFO:dill:D4: <dict object at 0x7f9be5bbfb40>
INFO:dill:Ce: <cell at 0x7f9be58a6be8: str object at 0x7f9bf418c7b0>
INFO:dill:F2: <function _create_cell at 0x7f9bed9d2578>
INFO:dill:Ce: <cell at 0x7f9be58a6bb0: type object at 0x251b6d0>
INFO:dill:T2: <class '__main__.NewCls'>
INFO:dill:F2: <function _create_type at 0x7f9bed9d2230>
INFO:dill:T1: <type 'type'>
INFO:dill:D2: <dict object at 0x7f9bdf2266e0>
INFO:dill:F1: <function f at 0x7f9be1c106e0>
INFO:dill:D4: <dict object at 0x7f9be5bbfb40>
INFO:dill:Ce: <cell at 0x7f9be58a6bb0: type object at 0x251b6d0>
INFO:dill:T2: <class '__main__.NewCls'>
INFO:dill:D2: <dict object at 0x7f9bdf370050>
INFO:dill:F1: <function f at 0x7f9be1c106e0>
INFO:dill:D4: <dict object at 0x7f9be5bbfb40>
INFO:dill:Ce: <cell at 0x7f9be58a6bb0: type object at 0x251b6d0>
INFO:dill:T2: <class '__main__.NewCls'>
INFO:dill:D2: <dict object at 0x7f9bdf222a28>
INFO:dill:F1: <function f at 0x7f9be1c106e0>
INFO:dill:D4: <dict object at 0x7f9be5bbfb40>
INFO:dill:Ce: <cell at 0x7f9be58a6bb0: type object at 0x251b6d0>
INFO:dill:T2: <class '__main__.NewCls'>

I will try to find a minimal test case to reproduce this too.

@matsjoyce
Copy link
Contributor

In the test above if I change line 9 to setattr(cls, 'methodname', proto_method) instead of explicitly using MethodType then the test passes.

Interesting. Maybe this is one of the things they simplified in python3, for reasons like this.

@matsjoyce
Copy link
Contributor

I think in essence, this is a problem with pickle, not dill, as when saving an object, pickle resolved the dependencies before marking the object as memorised, which means that circular references are not well handled.

@mmckerns
Copy link
Member

This may mean digging into exactly how the object is created by python, and seeing if there's a way to rebuild it from any component parts that are pickleable. In this case, looking at what exactly setattr is doing, seeing if there's a simple way to detect that setattr was used, and then building a function to replicate the object construction.

@matsjoyce
Copy link
Contributor

I've found the difference with the first issue:

>>> import types
>>> def f(): pass
... 
>>> class A:
...     def f():pass
... 
>>> class A2: pass
... 
>>> f2=types.MethodType(f, None, A2)
>>> A2.f=f2
>>> A.__dict__["f"]
<function f at 0x7f3d83ec7b90>
>>> A.f
<unbound method A.f>
>>> A2.__dict__["f"]
<unbound method A2.f>
>>> A2.f
<unbound method A2.f>
>>> 

@mmckerns
Copy link
Member

Ach.. that's bordering on being a python bug!

A loose plan to serialize A2 might be:

  1. Identify unbound methods in the __dict__
  2. serialize the methods as: A2.f.im_func (or possibly A2.f) in A2, or "with A2" as a tuple of (A2, dict) where dict has string names (i.e. "f") as keys and the objects as values
  3. in unpickling, deserialize A2, and then "reapply" the "special" methods exactly how they were built.

@mattja
Copy link
Author

mattja commented Jul 23, 2014

Turns out this has indeed been fixed in python3. The instancemethod type has been removed and all unbound methods are now type function.
setattr(cls, 'methodname', f) is the only idom that works in both versions so it seems people should avoid types.MethodType for portable code in any case. I'll focus on trying to isolate the second issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants