RFont.copy() links sub-objects in `info`, rather than copying them

[ryanbugden]

Issue:

I ran into a situation in which changing data on a copy of a font was affecting the data on the source font.

f1 = NewFont()
f1.info.postscriptBlueValues = [-10, 0, 50, 60]

f2 = f1.copy()
f2.info.postscriptBlueValues[0] = -2
f2.openInterface()

print(f1.info.postscriptBlueValues)
print(f2.info.postscriptBlueValues)

Output:

[-2, 0, 50, 60]
[-2, 0, 50, 60]

Workaround:

The only workaround I was able to find was copying the list data itself. Should this be necessary?
I'm also reassigning rather than editing directly (that part is just me correcting bad practice above).

f1 = NewFont()
f1.info.postscriptBlueValues = [-10, 0, 50, 60]

f2 = f1.copy()
values = f1.info.postscriptBlueValues.copy()
values[0] = -2
f2.info.postscriptBlueValues = values
f2.openInterface()

print(f1.info.postscriptBlueValues)
print(f2.info.postscriptBlueValues)

Output:

[-10, 0, 50, 60]
[-2, 0, 50, 60]

cc @typesupply

[benkiel]

@typesupply perhaps it is as simple as using deepcopy in base.copyData

[typesupply]

@benkiel Hmmm. Yeah, I think this could do it without causing breakage:

def copyData(self, source):
    """
    Subclasses may override this method.
    If so, they should call the super.
    """
    for attr in self.copyAttributes:
        selfValue = getattr(self, attr)
        sourceValue = getattr(source, attr)
        if isinstance(selfValue, BaseObject):
            selfValue.copyData(sourceValue)
        else:
            setattr(self, attr, deepcopy(sourceValue))

This way the deepcopy is only applied if the object type isn't BaseObject. We could make it even safer by doing:

def copyData(self, source):
    """
    Subclasses may override this method.
    If so, they should call the super.
    """
    for attr in self.copyAttributes:
        selfValue = getattr(self, attr)
        sourceValue = getattr(source, attr)
        if isinstance(selfValue, BaseObject):
            selfValue.copyData(sourceValue)
        else:
            if hasattr(sourceValue, "__deepcopy__"):
                sourceValue = deepcopy(sourceValue)
            setattr(self, attr, sourceValue)

These implementations might add a bit of overhead, but I'm sure the copy module is as optimized as it can be. I'm sorry I missed this when I built BaseInfo.

[benkiel]

Ok, I've added a test (#765) and done the second of the above against the v1 branch (#764). Test there failed.

[benkiel]

Going with the first option, the tests pass.

[benkiel]

Fixed in #765

[ryanbugden]

Thank you both!