gumyr / build123d

A python CAD programming library
Apache License 2.0
402 stars 75 forks source link

Solid.is_solid is broken #96

Closed bernhard-42 closed 1 year ago

bernhard-42 commented 1 year ago

Currently it is implemented as

class Solid(Shape, Mixin3D):
        if hasattr(obj, "shape_type"):
            if obj.shape_type == Solid or (
                obj.shape_type == Compound and len(obj.solids()) > 0
            ):
                return True
        return False

But I guess it should be

class Solid(Shape, Mixin3D):
        if hasattr(obj, "shape_type"):
            if obj.shape_type() == "Solid" or (
                obj.shape_type() == "Compound" and len(obj.solids()) > 0
            ):
                return True
        return False

since shape_type is a method and no property. And it returns a string.

gumyr commented 1 year ago

Thanks - this is clearly an error. However, I don't really see the utility of is_solid at all. Here are some alternatives:

from build123d import *

print(f"{isinstance(Solid.make_box(1,1,1),Solid)=}")
print(f"{isinstance(Face.make_rect(1,1),Solid)=}")
print(f"{len(Compound(children=[Solid.make_box(1,1,1)]).solids())=}")
print(f"{len(Compound(children=[Face.make_rect(1,1)]).solids())=}")
isinstance(Solid.make_box(1,1,1),Solid)=True
isinstance(Face.make_rect(1,1),Solid)=False
len(Compound(children=[Solid.make_box(1,1,1)]).solids())=1
len(Compound(children=[Face.make_rect(1,1)]).solids())=0

I propose elimination of is_solid - do you see any reason to keep it?

bernhard-42 commented 1 year ago

Since it was there I used it and started to wonder why it always returns False ...

I agree, we don't really need it.

gumyr commented 1 year ago

Okay, good - I'll remove it. A little less code to maintain.