bernhard-42 / vscode-ocp-cad-viewer

A viewer for OCP based Code-CAD (CadQuery, build123d) integrated into VS Code
Apache License 2.0
78 stars 9 forks source link

Array containing vector and tuple crashes show #67

Closed MatthiasJ1 closed 3 months ago

MatthiasJ1 commented 3 months ago
a = [Vector(1,2,3), (1,2,3)]
show_all()
bernhard-42 commented 3 months ago

I feel it is the right decision to raise an exception. What do you expect to happen? Silently assuming the since it is a 3-dim tuple that it is a Vector? I think I'd prefer raising an exception and the developer should take care to convert a tuple to a Vector. Since Vector(Vector) works, this can be done without effort

MatthiasJ1 commented 3 months ago

I agree that it would be a reasonable behavior to throw an error for show([Vector, Tuple]), but I think that does not extend to a=[Vector, Tuple]; show_all(). Since show_all attempts to process all in scope variables, you are expecting of the user to ensure every single variable that happens to exist to conform to the requirements of your code (how is the user supposed to know that a stray list in the format [Vector, Tuple] is going to cause the program to error out while any other combination works fine, as shown below. They might not even be attempting to show that list, it just happens to exist somewhere in their source file and now its giving an error) . show_all should just ignore invalid variables rather than throw an error or at most give a warning.


a = [Vector(1,2,3), Vector(1,2,3)]
show_all()
ok

a = [(1,2,3),(1,2,3)]
show_all()
ok

a = [(1,2,3), Vector(1,2,3)]
show_all()
ok

a = [Vector(1,2,3), (1,2,3)]
show_all()
RuntimeError: Cannot transform 1(<class 'int'>) to OCP
bernhard-42 commented 3 months ago

Makes sense. I added a conversion:

with BuildPart() as p:
    Box(1,1,1)

a = {
    "a": Vector(1, 2, 3),
    "b": [
        Pos(2, 2, 2) * Cylinder(1, 1),
        (1,2,3),
        p,
        p.part,
        {"c": Vector(5, 2, 3), "d": Pos(-3, 0, 0) * Box(1, 2, 3), "e": 123},
    ],
}

show_all({"a": a})

will now show image

and the output is

Unknown type <class 'tuple'> for name b[1]
Unknown type <class 'int'> for name e

As you can see the unknow types are simply excluded in the viewer

bernhard-42 commented 3 months ago

fixed in 2.2.2