SixTrack / pysixtrack

Reference sixtrack tracking engine in pure python
Apache License 2.0
2 stars 4 forks source link

Need to fix Line arguments to List instead of Tuple #45

Open aoeftiger opened 4 years ago

aoeftiger commented 4 years ago

... otherwise Line.append_element does not work.

line = pysixtrack.elements.Line()

line.append_element(
    pysixtrack.elements.Multipole(knl=[0, 1e-2]), 
    'multipole')

... throws a

AttributeError                            Traceback (most recent call last)
<ipython-input-22-7a02c560d482> in <module>
      3 line.append_element(
      4     pysixtrack.elements.Multipole(knl=[0, 1e-2]),
----> 5     'multipole')
      6 line.append_element(
      7     pysixtrack.elements.DriftExact(length=1),

~/gsi/git/pysixtrack/pysixtrack/line.py in append_element(self, element, name)
     83 
     84     def append_element(self, element, name):
---> 85         self.elements.append(element)
     86         self.element_names.append(name)
     87         # assert len(self.elements) == len(self.element_names)

AttributeError: 'tuple' object has no attribute 'append'

... while initialising the kwargs elements and element_names with empty List instances instead of the default empty Tuple instances resolves the problem:

pysixtrack.elements.Line(elements=[], element_names=[])
giadarol commented 4 years ago

Hi Adrian, the default values were originally lists, but this results in a the following unwanted behaviour:

class Test(object):
    def __init__(self, a=[]):
        self.a = a

t1 = Test()
t2 = Test()

print(t1.a, t2.a)
# gives: "[] []"

# I append something to the first object
t1.a.append(3)

print(t1.a, t2.a)
# gives: "[3] [3]!!!!"

In fact the .a members of the two objects point to the same list!!!

aoeftiger commented 4 years ago

Ah I see.. Since the tuples also lead to unwanted behaviour, I propose a different approach to resolve both problems at once..

Instantiating the list at the time of Test definition leads to the same list for all Test instances. If instead we use None as default value and instantiate a list at the time of the __init__ call, your mentioned problem will be gone. This also fixes the above issue I have reported.

class Test(object):
    def __init__(self, a=None):
        if a is None:
            self.a = []
        else:
            self.a = a

t1 = Test()
t2 = Test()

print(t1.a, t2.a)
# gives: "[] []"

# I append something to the first object
t1.a.append(3)

print(t1.a, t2.a)
# gives: "[3] []"
giadarol commented 4 years ago

This would force us to write explicit constructors for the pysixtrack elements, which we wanted to avoid.

The tuple was a way of telling the user that he can rebind line.elements but not append to it. Btw line is not the only case, the multipole behaves in the same way.

Probably the problem can be solved more elegantly modifying the baseclasses...