Repository: uqfoundation/dill

Issue:
Class attributes identical to inherited ones are wrongly deleted
In 0.3.7, attributes in the `__dict__` of a class are now deleted by `dill` if they have the same name and value as an attribute of a superclass. For example:

```python
import dill

class Foo:
    x = 1

class Bar(Foo):
    x = 1  # identical value as superclass

assert 'x' in Bar.__dict__  # passes

data = dill.dumps(Bar)
Bar2 = dill.loads(data)

assert 'x' in Bar2.__dict__  # fails with dill 0.3.7 (works with 0.3.6)
```

This behavior is caused by the `_get_typedict_type` function added in #577. Based on its docstring it seems the idea was to exclude inherited methods, but the code actually excludes any attribute whose value is identical to the inherited value. This is not safe (arguably even in the case of methods) because as the code above demonstrates, it changes the behavior of code which inspects the `__dict__` directly. Obviously the above code is a toy example, but we have real code which does this (basically to set a non-inherited flag on classes) and which was broken by `dill` 0.3.7.

In case it matters, I'm using Python 3.11.1 on macOS; we've also seen the problem under Python 3.10.12 on Ubuntu.

Discussion:
mmckerns: @anivegesana: you may want to check if this has any backward incompatible breakage for ABC classes of pickles of old ABC classes.

Patch:
diff --git a/dill/_dill.py b/dill/_dill.py
index d0cf543..3dcff76 100644
--- a/dill/_dill.py
+++ b/dill/_dill.py
@@ -1693,7 +1693,7 @@ def _get_typedict_type(cls, clsdict, attrs, postproc_list):
     for name, value in dict.items(clsdict):
         try:
             base_value = inherited_dict[name]
-            if value is base_value:
+            if value is base_value and hasattr(value, '__qualname__'):
                 to_remove.append(name)
         except KeyError:
             pass


Test patch:
diff --git a/dill/tests/test_abc.py b/dill/tests/test_abc.py
index 20fc28b..2b1bfda 100644
--- a/dill/tests/test_abc.py
+++ b/dill/tests/test_abc.py
@@ -77,7 +77,8 @@ def test_abc_non_local():
     # Set a property that StockPickle can't preserve
     instance.bar = lambda x: x**2
     depickled = dill.copy(instance)
-    assert type(depickled) is not type(instance)
+    assert type(depickled) is type(instance) #NOTE: issue #612, test_abc_local
+    #NOTE: dill.copy of local (or non-local) classes should (not) be the same?
     assert type(depickled.bar) is FunctionType
     assert depickled.bar(3) == 9
     assert depickled.sfoo() == "Static Method SFOO"
@@ -99,7 +100,7 @@ def test_abc_local():
     labc = dill.copy(LocalABC)
     assert labc is not LocalABC
     assert type(labc) is type(LocalABC)
-    # TODO should work like it does for non local classes
+    #NOTE: dill.copy of local (or non-local) classes should (not) be the same?
     # <class '__main__.LocalABC'>
     # <class '__main__.test_abc_local.<locals>.LocalABC'>
 
diff --git a/dill/tests/test_classdef.py b/dill/tests/test_classdef.py
index 8ad17cb..57e6d58 100644
--- a/dill/tests/test_classdef.py
+++ b/dill/tests/test_classdef.py
@@ -276,6 +276,54 @@ def test_enummeta():
     assert dill.copy(HTTPStatus.OK) is HTTPStatus.OK
     assert dill.copy(enum.EnumMeta) is enum.EnumMeta
 
+def test_inherit(): #NOTE: see issue #612
+    class Foo:
+        w = 0
+        x = 1
+        y = 1.1
+        a = ()
+        b = (1,)
+        n = None
+
+    class Bar(Foo):
+        w = 2
+        x = 1
+        y = 1.1
+        z = 0.2
+        a = ()
+        b = (1,)
+        c = (2,)
+        n = None
+
+    Baz = dill.copy(Bar)
+
+    import platform
+    is_pypy = platform.python_implementation() == 'PyPy'
+    assert Bar.__dict__ == Baz.__dict__
+    # ints
+    assert 'w' in Bar.__dict__ and 'w' in Baz.__dict__
+    assert Bar.__dict__['w'] is Baz.__dict__['w']
+    assert 'x' in Bar.__dict__ and 'x' in Baz.__dict__
+    assert Bar.__dict__['x'] is Baz.__dict__['x']
+    # floats
+    assert 'y' in Bar.__dict__ and 'y' in Baz.__dict__
+    same = Bar.__dict__['y'] is Baz.__dict__['y']
+    assert same if is_pypy else not same
+    assert 'z' in Bar.__dict__ and 'z' in Baz.__dict__
+    same = Bar.__dict__['z'] is Baz.__dict__['z']
+    assert same if is_pypy else not same
+    # tuples
+    assert 'a' in Bar.__dict__ and 'a' in Baz.__dict__
+    assert Bar.__dict__['a'] is Baz.__dict__['a']
+    assert 'b' in Bar.__dict__ and 'b' in Baz.__dict__
+    assert Bar.__dict__['b'] is not Baz.__dict__['b']
+    assert 'c' in Bar.__dict__ and 'c' in Baz.__dict__
+    assert Bar.__dict__['c'] is not Baz.__dict__['c']
+    # None
+    assert 'n' in Bar.__dict__ and 'n' in Baz.__dict__
+    assert Bar.__dict__['n'] is Baz.__dict__['n']
+
+
 if __name__ == '__main__':
     test_class_instances()
     test_class_objects()
@@ -289,3 +337,4 @@ if __name__ == '__main__':
     test_origbases()
     test_metaclass()
     test_enummeta()
+    test_inherit()