0.3.7 incorrectly pickles the class definition for module/class with the same name about dill HOT 7 CLOSED

FYI - possible fix for you to evaluate is the following. It appears to fix the issue on my end and I believe it passes tests. But I don't understand the rational for why it was done this way in the first place so it would be good for you to look at. If you'd rather have this as a PR, let me know and I'll do that.

def _import_module(import_name, safe=False):
    try:
        # attempt to fix git bug #634.
        m = sys.modules.get(import_name)
        if m:
           return m
        #if import_name.startswith('__runtime__.'):
        #    return sys.modules[import_name]
        elif '.' in import_name:
            items = import_name.split('.')
            module = '.'.join(items[:-1])
            obj = items[-1]
        else:
            return __import__(import_name)
        return getattr(__import__(module, None, None, [obj]), obj)
    except (ImportError, AttributeError, KeyError):
        if safe:
            return None
        raise

from dill.

Thanks for reporting, and the follow-up on this. I'll have a look... and yes, I'll make a PR if you don't.

from dill.

Slight modification:

foo/__init__.py
    from .bar import bar, zap

foo/bar.py:
   class bar:
      pass

   class zap:
      pass

With the above, I don't think there's a difference for foo.bar.zap versus foo.bar.bar. So, it would seem the module/class having the same name doesn't matter. Also, I think this is the expected behavior -- dill makes a copy of the class instead of referencing it like pickle would. The warning is a bit annoying, but innocuous.

Can you tell me what you think the behavior should be?

from dill.

I'm not at a computer where I can check this. But I assume "with the above" means my hack/fix. That could be true - I didn't test every combination. The fix I proposed may not be correct and/or complete.

However that doesn't mean this isn't a bug and IMO it's a serious bug. The problem is that I have code that unpickles something and then says:

obj = dill.loads( data )
if isinstance( obj, foo.bar ):
   ...  do something

And the isinstance check will fail because dill made a clone of the class. So the unpickled object is no longer the same type as the object that was pickled. BTW regular pickle handles this case just fine - I don't know what it's doing but it correctly finds the package and unpickles to the original type.

from dill.

Currently, the expected behavior is that dill makes a copy of the class, and doesn't pickle a reference unless it's requested that the class is pickled by reference. I initially thought that you were reporting that the bug was the warning being thrown, but if it's that dill is not pickling by reference... then this is the expected behavior.

This enables dill to deal with all sorts of different cases, such as the class being dynamically modified.

from dill.

Sorry - I didn't realize that was the desired behavior. I doubt it matters at this point but FYI in every use case we have (saving data, sending functions and data to parallel engines) it's really bad to make a copy of the class unless there is no other way to pickle it. I guess we'll try to figure out how to suppress that error and wrap the library to set byRef=True for all of our calls.

from dill.

No worries. Also, those are the most common dill use cases, I believe. If you want classes to be pickled by reference in all cases, you can adjust the global settings. dill.settings['byref'] = True. -- then you won't need to use the keyword in the dump.

from dill.