bug in constructor with default value. [SOLVED]

[VladVons]

I found a bug in constructor with default value.
In last line aData should be [], but not a 100.
This problem also occures on Linux, Windows.

Code: Select all

import sys

class TClass1():
    def __init__(self, aName: str, aData: list = []):
        print('aName:', aName, 'aData:', aData)
        self.Data = aData


print()
print('python ver:', sys.version_info)

#--------------
print()
c1 = TClass1('ok')
c1.Data.append(100)

c2 = TClass1('ok', [1,2,3])

c3 = TClass1('err. aData cant be 100 !!!')

----- result ---:
python ver: (3, 4, 0)
aName: ok aData: []
aName: ok aData: [1, 2, 3]
aName: err. aData cant be 100 !!! aData: [100]

[rkompass]

The 'problem' even exists on my ordinary python:

Code: Select all

Python 3.10.2 (/usr/bin/python)
>>> %Run test_class1.py

python ver: sys.version_info(major=3, minor=10, micro=2, releaselevel='final', serial=0)

aName: ok aData: []
aName: ok aData: [1, 2, 3]
aName: err. aData cant be 100 !!! aData: [100]

My interpretation: By

Code: Select all

self.Data = aData

you introduce a reference to an object (aData), which continues to exist while the scope of aData is lost. aData was an argument and thus has only meaning within the function body ( __init()__ ).
Using

Code: Select all

self.Data = aData.copy()

the problem is gone.

[pythoncoder]

To put this another way, the line

Code: Select all

        self.Data = aData

sets .Data to be a reference to the default arg aData. So when you modify .Data with

Code: Select all

c1.Data.append(100)

you are also modifying aData. This means that when you create a new instance of TClass1 with the default list, the instance acquires the new default value. Here is a minimal demo:

Code: Select all

>>> a = TClass1('foo')
aName: foo aData: []
>>> a.Data.append(42)
>>> b = TClass1('bar')
aName: bar aData: [42]
>>> 

Default list args in Python are objects which are created once at compile time. This property can be useful in controlling allocation, but has its hazards. As @rkompass says, performing copy ensures that .Data is separate from the default list object.

[VladVons]

the responsibility of using 'copy()' lays on python byte compiler.
to reduce data segment the interpreter should control itself uniq values stored in hash table.
to best unerstanding see internal id() function.

though, the example with 'str' type works correct.

Code: Select all

class TClass1():
    def __init__(self, aName: str, aData: str = ''):
        print('aName:', aName, 'aData:', aData)
        self.Data = aData

print()
c1 = TClass1('ok.')
c1.Data = 'hello'

c2 = TClass1('ok.', 'world')

c3 = TClass1('ok. aData is empty')

--- result ---
aName: ok. aData: 
aName: ok. aData: world
aName: ok. aData is empty aData: 

[dhylands]

Using [] or {} as default arguments is almost always a mistake. See this blog post which describes why in more detail: https://nikos7am.com/posts/mutable-default-arguments/

You’d be better to use a default value of None and then inside your function do something like:

Code: Select all

 if arg is None:
     arg = []

This will allocate a new empty array each time the function is called rather than reusing the array that was allocated when the function was defined.

[pythoncoder]

There is a MicroPython-specific case where a mutable list default offers a useful optimisation. That is where a hard ISR requires a fixed-length buffer. You can't allocate it in the ISR. Options are a global buffer (slow) or a buffer as a bound variable. The other way is to define the buffer as a default arg: it is allocated once at compile time.

This is safe so long as there is only one instance of the ISR. If instances of the ISR are allocated to more than one interrupt, you need to be aware that there is one buffer shared between instances. Not usually what you want...