SolidCode / SolidPython

A python frontend for solid modelling that compiles to OpenSCAD
1.13k stars 177 forks source link

print warnings or error when a scoped operator is used without a scope #192

Closed maxlem closed 2 years ago

maxlem commented 2 years ago

Hi, thanks for your project!

I was stuck for a while transitioning code from scad to solidpython. I tend to open curly braces on the following line, replacing them with braces produces valid python code but surprizing results:

from solid import *

d = difference()
(
    cube(10),
    sphere(15)
)
scad_render_to_file(d, "tmp.scad")

produces

difference();

since python does not call operator() on difference() in this case.

I think one could easily detect such a use case and warn the user.

etjones commented 2 years ago

Hmm. That's a situation I haven't seen before, but I can see how it created some confusion.

This hasn't come up in the previous decade of SolidPython, so I confess I'm hesitant to make code changes off the bat. There are situations in which you reasonably would want to use an operator without a scope, like:

a = union()
a += cube(10)
a += translate([10,0,0])(sphere(10))

So it's not quite as simple as checking for presence/absence of following scope and then printing a warning. For the moment, I think I'm going to call this a wontfix.

In your case, I can think of a couple workarounds if you have a bunch of existing SCAD code you want to hold onto. First, you can use that SCAD directly with mod_name = import_scad('path_to_file.scad'); a = mod_name.func_name(args). Second, if you were bulk-converting .SCAD files, you could either surround blocks like your example with parens:


    # renders to OpenSCAD as `difference();`, a no-op
    no_parens = difference()
    (
        cube(10),
        sphere(15)
    )

    # Renders to OpenSCAD as you'd expect
    with_parens = (difference()
    (
        cube(10),
        sphere(15)
    )
    )

Or use a multi-line regex to transform all those BSD-style dangling braces into K&R-style line-end braces.

Because Python is whitespace sensitive and line breaks are [often] syntactic, you're likely to have a bad time with any indentation style that doesn't start on the same line as a statement.

maxlem commented 2 years ago

right now I just do

    no_parens = difference()(
        cube(10),
        sphere(15)
    )

but any operator without child (operand) is suspicious, and, without the dummy first pair of braces, it would never occur. What I mean is that

    no_parens = difference
(
        cube(10),
        sphere(15)
)

... would have failed at syntax check.

I might put togheter a MR if it bites me another time

jeff-dh commented 2 years ago
>>> from solid import *
>>> orig_render = difference._render
>>> 
>>> def warning_render(self, render_holes=False):
...     if len(self.children) == 0:
...             print("WARNING")
...     return orig_render(self, render_holes)
... 
>>> difference._render = warning_render
>>> 
>>> d = difference()
>>> scad_render(d)
WARNING
'\n\ndifference();'
>>> 
>>> d = cube() - sphere()
>>> scad_render(d)
'\n\ndifference() {\n\tcube();\n\tsphere();\n}'
>>> 

You would need to overwrite the _render function for each object you want to use this warning mechanism, you can't overwrite OpenSCADObject._render, because there are objects that don't have children (e.g. cube or sphere).

maxlem commented 2 years ago

Thanks jeff! I would have to do that for all the operators, as they share the same pitfall.

jeff-dh commented 2 years ago

I know, but.....:

>>> from solid import *
>>> def patch_operator(op):
...     orig_render = op._render
...     def warning_render(self, render_holes=False):
...             if len(self.children) == 0:
...                     print("WARNING")
...             return orig_render(self, render_holes)
...     op._render = warning_render
... 
>>> for i in [difference, union, intersection]:
...     patch_operator(i)
... 
>>> u = union()
>>> scad_render(u)
WARNING
'\n\nunion();'
>>> scad_render(cube() + sphere())
'\n\nunion() {\n\tcube();\n\tsphere();\n}'