Skip to content

Commit

Permalink
Rewrite copy and deepcopy implementations
Browse files Browse the repository at this point in the history
* Use internal copy() and deepCopy() methods rather than pickling.
* Have separate implementations for PropertySet and PropertyList
  (since the inheritance is about to be broken).
* Remove name override from the pickle routines since they no
  longer are used for copying.
  • Loading branch information
timj committed Sep 26, 2018
1 parent c370964 commit eec3fdc
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
from ..dateTime import DateTime


def getPropertySetState(container, asLists=False, names=None):
def getPropertySetState(container, asLists=False):
"""Get the state of a PropertySet in a form that can be pickled.
Parameters
Expand All @@ -46,8 +46,6 @@ def getPropertySetState(container, asLists=False, names=None):
asLists : `bool`, optional
If False, the default, `tuple` will be used for the contents. If true
a `list` will be used.
names : `list` or `tuple`
Override the default list of names with this subset.
Returns
-------
Expand All @@ -62,12 +60,7 @@ def getPropertySetState(container, asLists=False, names=None):
- value: the data for the item, in a form compatible
with the set method named by ``elementTypeName``
"""
if names is None:
# All top level names: this allows hierarchical PropertySet and
# PropertyList to be represented as their own entities. Without
# this a PropertyList inside a PropertySet loses all comments
# and becomes a PropertySet.
names = container.names(topLevelOnly=True)
names = container.names(topLevelOnly=True)
sequence = list if asLists else tuple
return [sequence((name, _propertyContainerElementTypeName(container, name),
_propertyContainerGet(container, name, returnStyle=ReturnStyle.AUTO)))
Expand Down Expand Up @@ -100,11 +93,12 @@ def getPropertyListState(container, asLists=False):
- comment (a `str`): the comment. This item is only present
if ``container`` is a PropertyList.
"""
names = container.getOrderedNames()
sequence = list if asLists else tuple
return [sequence((name, _propertyContainerElementTypeName(container, name),
_propertyContainerGet(container, name, returnStyle=ReturnStyle.AUTO),
container.getComment(name)))
for name in container.getOrderedNames()]
for name in names]


def setPropertySetState(container, state):
Expand Down Expand Up @@ -479,11 +473,16 @@ def __eq__(self, other):
return True

def __copy__(self):
# Provide a copy because by default __reduce__ is used and that
# will not shallow copy properly, we therefore use the same
# pickling code but restrict the names
state = getPropertySetState(self, names=self.names(topLevelOnly=True))
return _makePropertySet(state)
# Copy without having to go through pickle state
ps = PropertySet()
for itemName in self:
ps.copy(itemName, self, itemName)
return ps

def __deepcopy__(self, memo):
result = self.deepCopy()
memo[id(self)] = result
return result

def __contains__(self, name):
# Do not use exists() because that includes "."-delimited names
Expand Down Expand Up @@ -714,11 +713,16 @@ def __eq__(self, other):
return True

def __copy__(self):
# Provide a copy because by default __reduce__ is used and that
# will not shallow copy properly, we therefore use the same
# pickling code but restrict the names
state = getPropertyListState(self, names=self.getOrderedNames())
return _makePropertyList(state)
# Copy without having to go through pickle state
pl = PropertyList()
for itemName in self:
pl.copy(itemName, self, itemName)
return pl

def __deepcopy__(self, memo):
result = self.deepCopy()
memo[id(self)] = result
return result

def __iter__(self):
for n in self.getOrderedNames():
Expand Down
40 changes: 36 additions & 4 deletions tests/test_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,32 @@ def setUp(self):
self.ps = ps
self.pl = pl

def testCopyPropertySet(self):
def testShallowCopyPropertySet(self):
shallow = copy.copy(self.ps)
self.assertIsInstance(shallow, lsst.daf.base.PropertySet)
self.assertIn("ps2", shallow)
self.assertEqual(shallow, self.ps)

# Modifying the attached property set should change both views
ps2 = self.ps.getScalar("ps2")
ps2s = shallow.getScalar("ps2")
self.assertEqual(ps2, ps2s)

ps2["int2"] = 2017
self.assertEqual(ps2, ps2s)

def testDeepCopyPropertySet(self):
deep = copy.deepcopy(self.ps)
self.assertIsInstance(deep, lsst.daf.base.PropertySet)
self.assertEqual(deep, self.ps)
del deep["ps2"]
self.assertNotIn("ps2", deep)
self.assertIn("ps2", self.ps)

# Modifying the contents of ps2 should not affect the original
ps2 = self.ps.getScalar("ps2")
ps2d = deep.getScalar("ps2")
self.assertEqual(ps2, ps2d)

ps2["int2"] = 2017
self.assertNotEqual(ps2, ps2d)

def testDictPropertySet(self):
container = self.ps
Expand Down Expand Up @@ -155,6 +169,24 @@ def testDictPropertyList(self):
container["dict"] = {"a": 1, "b": 2}
self.assertEqual(container.getScalar("dict.b"), 2)

def testCopyPropertyList(self):
# For PropertyList shallow copy and deep copy are identical
shallow = copy.copy(self.pl)
self.assertIsInstance(shallow, lsst.daf.base.PropertyList)
self.assertIn("dt", shallow)
self.assertIn("int", shallow)
self.assertEqual(shallow, self.pl)
del shallow["dt"]
self.assertNotIn("dt", shallow)
self.assertIn("dt", self.pl)

deep = copy.deepcopy(self.pl)
self.assertIsInstance(deep, lsst.daf.base.PropertyList)
self.assertEqual(deep, self.pl)
del deep["dt"]
self.assertNotIn("dt", deep)
self.assertIn("dt", self.pl)


if __name__ == '__main__':
unittest.main()

0 comments on commit eec3fdc

Please sign in to comment.