Repository: spacetelescope/asdf
Pull request #788 (2020-04-29 16:34:03)

Fix deepcopy of TaggedList
The current behavior:

```
In [1]: from asdf.tagged import TaggedList; from copy import deepcopy

In [2]: foo = TaggedList([1, 2, 3])

In [3]: deepcopy(foo)
Out[3]: [1, 2, 3, 1, 2, 3]

In [4]: deepcopy(deepcopy(foo))
Out[4]: [1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3]
```

This has something to do with the `TaggedList` class inheriting from both `list` and `UserList`, but we can't drop `list`, because code elsewhere is probably relying upon this:

```
In [5]: isinstance(foo, list)
Out[5]: True
```

Which isn't the case when we inherit from `UserList` only -- users are expected to check `collections.abc.Sequence` instead.  I'm in favor of dropping list eventually, but that seems like a 3.0 change.

This PR overrides `__deepcopy__` to correct the issue with `UserList`.  It does the same on `UserDict`, even though there wasn't a problem there, since the custom `__deepcopy__` ended up being ~ 60% faster than whatever was happening without it.  Overriding `__copy__` offers no improvement, so I just left that alone.

Thanks to @jdavies-st for suggesting both `__deepcopy__` and profiling `UserDict`!


Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 0c6e914a..a73b1d88 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -4,6 +4,9 @@
 - Fix bug preventing diff of files containing ndarray-1.0.0
   objects in simplified form. [#786]
 
+- Fix bug causing duplicate elements to appear when calling
+  ``copy.deepcopy`` on a ``TaggedList``. [#788]
+
 2.6.0 (2020-04-22)
 ------------------
 
diff --git a/asdf/tagged.py b/asdf/tagged.py
index ee6ea683..baa7d561 100644
--- a/asdf/tagged.py
+++ b/asdf/tagged.py
@@ -32,6 +32,7 @@ is not intended to be exposed to the end user.
 """
 
 from collections import UserDict, UserList, UserString
+from copy import deepcopy, copy
 
 
 __all__ = ['tag_object', 'get_tag']
@@ -63,6 +64,14 @@ class TaggedDict(Tagged, UserDict, dict):
                 self.data == other.data and
                 self._tag == other._tag)
 
+    def __deepcopy__(self, memo):
+        data_copy = deepcopy(self.data, memo)
+        return TaggedDict(data_copy, self._tag)
+
+    def __copy__(self):
+        data_copy = copy(self.data)
+        return TaggedDict(data_copy, self._tag)
+
 
 class TaggedList(Tagged, UserList, list):
     """
@@ -81,6 +90,14 @@ class TaggedList(Tagged, UserList, list):
                 self.data == other.data and
                 self._tag == other._tag)
 
+    def __deepcopy__(self, memo):
+        data_copy = deepcopy(self.data, memo)
+        return TaggedList(data_copy, self._tag)
+
+    def __copy__(self):
+        data_copy = copy(self.data)
+        return TaggedList(data_copy, self._tag)
+
 
 class TaggedString(Tagged, UserString, str):
     """


Test patch:
diff --git a/asdf/tests/test_tagged.py b/asdf/tests/test_tagged.py
new file mode 100644
index 00000000..4a55bf62
--- /dev/null
+++ b/asdf/tests/test_tagged.py
@@ -0,0 +1,82 @@
+from copy import deepcopy, copy
+
+from asdf.tagged import TaggedList, TaggedDict, TaggedString
+
+
+def test_tagged_list_deepcopy():
+    original = TaggedList([0, 1, 2, ["foo"]], "tag:nowhere.org:custom/foo-1.0.0")
+    result = deepcopy(original)
+    assert result == original
+    assert result.data == original.data
+    assert result._tag == original._tag
+    original.append(4)
+    assert len(result) == 4
+    original[3].append("bar")
+    assert len(result[3]) == 1
+
+
+def test_tagged_list_copy():
+    original = TaggedList([0, 1, 2, ["foo"]], "tag:nowhere.org:custom/foo-1.0.0")
+    result = copy(original)
+    assert result == original
+    assert result.data == original.data
+    assert result._tag == original._tag
+    original.append(4)
+    assert len(result) == 4
+    original[3].append("bar")
+    assert len(result[3]) == 2
+
+
+def test_tagged_list_isinstance():
+    value = TaggedList([0, 1, 2, ["foo"]], "tag:nowhere.org:custom/foo-1.0.0")
+    assert isinstance(value, list)
+
+
+def test_tagged_dict_deepcopy():
+    original = TaggedDict({"a": 0, "b": 1, "c": 2, "nested": {"d": 3}}, "tag:nowhere.org:custom/foo-1.0.0")
+    result = deepcopy(original)
+    assert result == original
+    assert result.data == original.data
+    assert result._tag == original._tag
+    original["e"] = 4
+    assert len(result) == 4
+    original["nested"]["f"] = 5
+    assert len(result["nested"]) == 1
+
+
+def test_tagged_dict_copy():
+    original = TaggedDict({"a": 0, "b": 1, "c": 2, "nested": {"d": 3}}, "tag:nowhere.org:custom/foo-1.0.0")
+    result = copy(original)
+    assert result == original
+    assert result.data == original.data
+    assert result._tag == original._tag
+    original["e"] = 4
+    assert len(result) == 4
+    original["nested"]["f"] = 5
+    assert len(result["nested"]) == 2
+
+
+def test_tagged_dict_isinstance():
+    value = TaggedDict({"a": 0, "b": 1, "c": 2, "nested": {"d": 3}}, "tag:nowhere.org:custom/foo-1.0.0")
+    assert isinstance(value, dict)
+
+
+def test_tagged_string_deepcopy():
+    original = TaggedString("You're it!")
+    original._tag = "tag:nowhere.org:custom/foo-1.0.0"
+    result = deepcopy(original)
+    assert result == original
+    assert result._tag == original._tag
+
+
+def test_tagged_string_copy():
+    original = TaggedString("You're it!")
+    original._tag = "tag:nowhere.org:custom/foo-1.0.0"
+    result = copy(original)
+    assert result == original
+    assert result._tag == original._tag
+
+
+def test_tagged_string_isinstance():
+    value = TaggedString("You're it!")
+    assert isinstance(value, str)