Shallow copy instead of new object in Python

Question

I'm trying to create a new object of a class inside this same class, however instead of creating a whole new object it simply creates a new reference to the same object I'm currently working in.

So if I change the value of one object, it changes the value in the other as well - even though I probably should have 2 completely different objects.

Using the copy.deepcopy() method fixes the issue with the reference, but I guess that it should work differently as well.

How does my code behave in this specific implementation? Is there a reason for it to create a shallow copy of the same object even though the code probably should create a new instance of it?

This is a slightly reduced code snippet:

class Vec4():
    def __init__(self, x = 0, y = 0, z = 0, w = 0):
        self.values = [x,y,z,w]

    def __str__(self):
        return str(self.values[0]) + ' ' + str(self.values[1]) + ' ' + str(self.values[2]) + ' ' + str(self.values[3])

    def setValue(self, index, value):
        self.values[index] = value


    def scalar(self, vector):
        """returns the result of the scalar multiplication"""
        result = 0
        for u in range(4):
            result += self.values[u] * vector.values[u]
        return result




class Matrix4():
    def __init__(self, row1 = Vec4(), row2 = Vec4(), row3 = Vec4(), row4 = Vec4()):
        self.m_values = [row1,row2,row3,row4]
        self.trans_values = [Vec4(),Vec4(),Vec4(),Vec4()]
        self.set_transp_matrix()

    def __str__(self):
        return self.m_values[0].__str__() + '\n' + self.m_values[1].__str__() + '\n' + self.m_values[2].__str__() + '\n' + self.m_values[3].__str__()

    def setIdentity(self):

        identity = Matrix4(Vec4(1,0,0,0),
                       Vec4(0,1,0,0),
                       Vec4(0,0,1,0),
                       Vec4(0,0,0,1))
        for i in range(4):
            for j in range(4):
                self.m_values[i].values[j] = identity.m_values[i].values[j]

    def set_transp_matrix(self):
         for t in range(4):
            for s in range(4):
                self.trans_values[t].values[s] = self.m_values[s].values[t]

    def get_trans_matrix(self):
        return self.trans_values[0].__str__() + '\n' + self.trans_values[1].__str__() + '\n' + self.trans_values[2].__str__() + '\n' + self.trans_values[3].__str__()

    def mulM(self, m):

        print(self, "\n")
        matrixResult = Matrix4()
        print(matrixResult, "\n")
        for row in range(4):  # rows of self
            for element in range(4):
                value = self.m_values[row].scalar(m.trans_values[element])
                matrixResult.m_values[row].setValue(element, value)
        return matrixResult


class ScaleMatrix(Matrix4):

    def __init__(self, m_scale = Vec4(1,1,1), *args, **kwargs):
        super(ScaleMatrix, self).__init__(*args, **kwargs)
        self.m_scale = m_scale
        self.update()

    def getScale(self):
        """Returns the scale vector, only x, y and z are relevant"""
        return self.m_scale

    def setScale(self, v):
        """Sets the scale vector, only x, y and z are relevant"""
        self.m_scale = v
        self.update()

    def update(self):
        """Calculates the scale matrix"""

        self.setIdentity()

        for i in range(3):
            self.m_values[i].values[i] = self.getScale().values[i]

        return self


if __name__ == "__main__":
    #Simple Constructor and Print
    a = Vec4(1,2,3,4)
    b = Vec4(5,6,7,8)
    c = Vec4(9,10,11,12)
    d = Vec4(13,14,15,16)


    A = Matrix4(a, b, c, d)
    D = ScaleMatrix()
    D.setScale(Vec4(3, 4, 5, 1))

    print(D.mulM(A))

The issue is in the class Matrix4, method mulM() where matrixResult = Matrix4() should create a whole new instance of Matrix4() (where all values of Vec4() should be 0) instead of simply copying the self object. The output of print shows the following:

3 0 0 0
0 4 0 0
0 0 5 0
0 0 0 1 

3 0 0 0
0 4 0 0
0 0 5 0
0 0 0 1 

3 6 51 672
20 64 508 6688
45 140 1170 15340
13 40 334 4396

So the second matrix should not be equal to the first one. However, the issue does not occur if I create a normal Matrix4() object instead of the ScaleMatrix() object which extends Matrix4() in the very end of the code snippet above.

Python v.3.6.4

While *shadow* copy sounds super cool, are you sure you don't mean *shallow* copy? — Olivier Melançon, May 19 '18 at 21:35
Yep, you are right. Thank you for the correction, I'll edit my post accordingly. — webFashion, May 19 '18 at 21:36

score 2 · Accepted Answer · answered May 19 '18 at 22:25

Python evaluates the default parameters when a function is defined. When you define:

class Matrix4():
    def __init__(self, row1 = Vec4() ...

you create a Vec4 instance, and this instance will now be used as default value each time this __init__ method gets called.

The first time you create a Matrix4 instance, __init__ will execute, and this instance gets refered to by the name row1

Then you have:

    self.m_values = [row1,row2,row3,row4]

So this instance is now refered to by self.m_values[0].

Later, in ScaleMatrix.update, you update this Vec4 instance:

for i in range(3):
            self.m_values[i].values[i] = self.getScale().values[i]

The next time you will call Matrix4.__init__ without parameters, the default value will be used, and this is the Vec4 that you just updated.

You have a similar behaviour when you use an empty list as default parameter, see “Least Astonishment” and the Mutable Default Argument .

The usual way to avoid this problem is to avoid using mutable objects as default parameters. You could do:

class Matrix4():
    def __init__(self, row1 = None, row2 = None, row3 = None, row4 = None):
        if row1 is None:
            row1 = Vec4()
        if row2 is None:
            row2 = Vec4()
        if row3 is None:
            row3 = Vec4()
        if row4 is None:
            row4 = Vec4()
        self.m_values = [row1,row2,row3,row4]
        self.trans_values = [Vec4(),Vec4(),Vec4(),Vec4()]
        self.set_transp_matrix()

which gives as output:

3 0 0 0
0 4 0 0
0 0 5 0
0 0 0 1 

0 0 0 0
0 0 0 0
0 0 0 0
0 0 0 0 

3 6 9 12
20 24 28 32
45 50 55 60
13 14 15 16

Thank you for your descriptive explanation! I do understand my issue a lot better now — webFashion, May 19 '18 at 23:07

score 1 · Answer 2 · answered May 19 '18 at 22:26

Check out Python constructor and default value and "Least Astonishment" and the Mutable Default Argument Your issue is fundamentally similar to the issue described in the above linked issues.

Also, when declaring a class, make it a subclass of object:

class Vec4(object):
class Matrix4(object):

Finally, to address your particular issue, remove defaults for the initializer. For example, do something like this:

class Matrix4():
    def __init__(self, row1=None, row2=None, row3=None, row4=None):
       if row1 is None:
           row1 = Vec4()
       if row2 is None:
           row2 = Vec4()
       if row3 is None:
           row3 = Vec4()
       if row4 is None:
           row4 = Vec4()
       self.m_values = [row1,row2,row3,row4]

Same goes for the default of ScaleMatrix argument m_scale:

class ScaleMatrix(Matrix4):
    def __init__(self, m_scale = Vec4(1,1,1), *args, **kwargs):

Shallow copy instead of new object in Python

2 Answers2