SolidCode / SolidPython

A python frontend for solid modelling that compiles to OpenSCAD
1.1k stars 172 forks source link

Infinite Recursion #150

Open ottojas opened 4 years ago

ottojas commented 4 years ago

Hit problem by accident. reduced code to this:

l=[s.sphere(1),s.cube([.5,.6,2])] l.append(l) l [<solid.objects.sphere object at 0x7f55db249eb8>, <solid.objects.cube object at 0x7f55db249f28>, [...]] sum(l)

.... GIANT SNIP .... File "/home/otto/.local/lib/python3.7/site-packages/solid/solidpython.py", line 246, in [self.add(c) for c in child] File "/home/otto/.local/lib/python3.7/site-packages/solid/solidpython.py", line 246, in add [self.add(c) for c in child] File "/home/otto/.local/lib/python3.7/site-packages/solid/solidpython.py", line 246, in [self.add(c) for c in child] File "/home/otto/.local/lib/python3.7/site-packages/solid/solidpython.py", line 241, in add if isinstance(child, (list, tuple)): RecursionError: maximum recursion depth exceeded while calling a Python object

Regards Otto

Cabalist commented 4 years ago

You are appending something to itself.

I think this would cause an exception in any python program.

ottojas commented 4 years ago

Actually python handles this very well.

k=[5,6.7] sum(k) 11.7

Here is how python handles it.

k.append(k) k [5, 6.7, [...]] sum(k) Traceback (most recent call last): File "", line 1, in TypeError: unsupported operand type(s) for +: 'float' and 'list' Rather than going into infinite recursion it produces an error.

There is a fix.

File "/home/otto/.local/lib/python3.7/site-packages/solid/solidpython.py", line 246, in [self.add(c) for c in child] File "/home/otto/.local/lib/python3.7/site-packages/solid/solidpython.py", line 241, in add if isinstance(child, (list, tuple)):

The code: if len(child) == 1 and isinstance(child[0], (list, tuple)): child = child[0] [self.add(c) for c in child]

Is too promiscuous.

The recursion originally occurred accidentally, the appending a list to itself is what was happening deep in code and I created the example specifically to show that this could happen. I don't think this is possible in openscad itself. But maybe I will try to see if I can get a self referential loop going there. I rather doubt it.

Solid python is a great program and I am porting a number of my projects to it and want to support it.. But it does still have a few raw edges.

Regards Otto

On Wed, 15 Jul 2020 14:09:35 -0700 Ryan Jarvis notifications@github.com wrote:

You are appending something to itself.

I think this would cause an exception in any python program.

ottojas commented 3 years ago

I can't find the issue to post my fix. Here it is:

In solidpython.py in the add method,

def add(self, child: Union["OpenSCADObject",Sequence["OpenSCADObject"]])

change the last line of:

    if isinstance(child, (list, tuple)):
        # __call__ passes us a list inside a tuple, but we only care
        # about the list, so skip single-member tuples containing lists
        if len(child) == 1 and isinstance(child[0], (list, tuple)):
            child = child[0]
        [self.add(c) for c in child]

to two lines:

    if isinstance(child, (list, tuple)):
        # __call__ passes us a list inside a tuple, but we only care
        # about the list, so skip single-member tuples containing lists
        if len(child) == 1 and isinstance(child[0], (list, tuple)):
            child = child[0]
            [self.add(c) for c in child]
        else:
            raise TypeError("unsupported operand type(s) for +:
            'solid.object' and 'list'") 

Which causes solid to treat it in the same manner as python and throw an error rather than an infinite recursion.

Regards Otto

Actually python handles this very well.

k=[5,6.7] sum(k)
11.7

Here is how python handles it.

k.append(k) k
[5, 6.7, [...]] sum(k)
Traceback (most recent call last): File "", line 1, in TypeError: unsupported operand type(s) for +: 'float' and 'list' Rather than going into infinite recursion it produces an error.

There is a fix.

File "/home/otto/.local/lib/python3.7/site-packages/solid/solidpython.py", line 246, in [self.add(c) for c in child] File "/home/otto/.local/lib/python3.7/site-packages/solid/solidpython.py", line 241, in add if isinstance(child, (list, tuple)):

The code: if len(child) == 1 and isinstance(child[0], (list, tuple)): child = child[0] [self.add(c) for c in child]

Is too promiscuous.

The recursion originally occurred accidentally, the appending a list to itself is what was happening deep in code and I created the example specifically to show that this could happen. I don't think this is possible in openscad itself. But maybe I will try to see if I can get a self referential loop going there. I rather doubt it.

Solid python is a great program and I am porting a number of my projects to it and want to support it.. But it does still have a few raw edges.

Regards Otto

On Wed, 15 Jul 2020 14:09:35 -0700 Ryan Jarvis notifications@github.com wrote:

You are appending something to itself.

I think this would cause an exception in any python program.

ottojas commented 3 years ago

Please ignore previous letter. I will go through the draconian github technique for submitting bugs next time.

Fix doesn't work, it breaks other parts.

Regards Otto

I can't find the issue to post my fix. Here it is:

In solidpython.py in the add method,

def add(self, child: Union["OpenSCADObject",Sequence["OpenSCADObject"]])

change the last line of:

    if isinstance(child, (list, tuple)):
        # __call__ passes us a list inside a tuple, but we only care
        # about the list, so skip single-member tuples containing
    lists if len(child) == 1 and isinstance(child[0], (list,
    tuple)): child = child[0]
        [self.add(c) for c in child]

to two lines:

    if isinstance(child, (list, tuple)):
        # __call__ passes us a list inside a tuple, but we only care
        # about the list, so skip single-member tuples containing
    lists if len(child) == 1 and isinstance(child[0], (list,
    tuple)): child = child[0]
            [self.add(c) for c in child]
        else:
            raise TypeError("unsupported operand type(s) for +:
            'solid.object' and 'list'") 

Which causes solid to treat it in the same manner as python and throw an error rather than an infinite recursion.

Regards Otto

Actually python handles this very well.

k=[5,6.7] sum(k)
11.7

Here is how python handles it.

k.append(k) k
[5, 6.7, [...]] sum(k)
Traceback (most recent call last): File "", line 1, in TypeError: unsupported operand type(s) for +: 'float' and 'list' Rather than going into infinite recursion it produces an error.

There is a fix.

File "/home/otto/.local/lib/python3.7/site-packages/solid/solidpython.py", line 246, in [self.add(c) for c in child] File "/home/otto/.local/lib/python3.7/site-packages/solid/solidpython.py", line 241, in add if isinstance(child, (list, tuple)):

The code: if len(child) == 1 and isinstance(child[0], (list, tuple)): child = child[0] [self.add(c) for c in child]

Is too promiscuous.

The recursion originally occurred accidentally, the appending a list to itself is what was happening deep in code and I created the example specifically to show that this could happen. I don't think this is possible in openscad itself. But maybe I will try to see if I can get a self referential loop going there. I rather doubt it.

Solid python is a great program and I am porting a number of my projects to it and want to support it.. But it does still have a few raw edges.

Regards Otto

On Wed, 15 Jul 2020 14:09:35 -0700 Ryan Jarvis notifications@github.com wrote:

You are appending something to itself.

I think this would cause an exception in any python program.