gumyr / build123d

A python CAD programming library
Apache License 2.0
512 stars 86 forks source link

BuildLine.wires() not equal to itself #372

Closed MatthiasJ1 closed 10 months ago

MatthiasJ1 commented 11 months ago

The following will fail. This check works with every other combination of Builder and compatible faces, wires, etc.

from build123d import *

with BuildLine() as p:
    Line((0,0), (1,0))
    assert(p.wires() == p.wires())
gumyr commented 11 months ago

This is expected. BuildLine stores its objects in line which is defined as self.line: Curve = None where Curve is a derived class of Compound. When wires is called the following is executed:

    def wires(self) -> ShapeList[Wire]:
        """wires - all the wires in this Shape"""
        return ShapeList([Wire(i) for i in self._entities(Wire.__name__)])

where you'll note that a new Wire object is created each time. Therefore, when directly comparing either a single Wire or a list of Wires they will not be the same object(s).

snoyer commented 11 months ago

Wouldn't Wire objects still be able to be compared by their .wrapped shape? Comparing a ShapeList could go something like:

    def __eq__(self, other: Any):
        return (
            isinstance(other, ShapeList)
            and len(self) == len(other)
            and all(a.wrapped.IsEqual(b.wrapped) for a, b in zip(self, other))
        )

Looks to me like the more problematic bit is in Curve.wires() where whole new TopoDS_Wire are created from geometry at every call.

gumyr commented 11 months ago

Most of the objects creates by BuildLine (or the same objects used with Algebra) are Edges therefore the Curve/Compound may not have any Wire objects at all so they need be created from Edges or the results will not be what users expect. In addition, comparing just the TopoDS_Wire objects would allows wires that were different colors (or have other attributes that are different) to match so the compare would need to include these attributes too.

MatthiasJ1 commented 11 months ago

Makes sense, thanks for the explanation.

MatthiasJ1 commented 11 months ago

Is there a way to check whether two wires are the same, not whether they are the same object?

a = Wire.make_wire([Edge.make_line((0,0), (1,0))])
b = Wire.make_wire([Edge.make_line((0,0), (1,0))])

print(f"{a == b = }")
print(f"{a.is_equal(b) = }")
print(f"{a.is_same(b) = }")
a == b = False
a.is_equal(b) = False
a.is_same(b) = False
snoyer commented 11 months ago

When all else fails pickle.dumps(a) == pickle.dumps(b)

gumyr commented 11 months ago

There are is_equal and is_same methods that just wrap the OCP IsEqual and IsSame methods but they are different ways to compare the underlying TShape (the base shape without location). I don't know of a way to actually compare to objects that are "the same" but not the same object, other than @snoyer's suggestion to compare the results of pickle (being able to pickle the b3d objects is a new capability). However, I don't know how reliable this method is even though it works with a simple test.

>>> from build123d import *
>>> import pickle
>>> w1 = Polyline((0, 0), (1, 0), (1, 1)).wire()
>>> w2 = Polyline((0, 0), (1, 0), (1, 1)).wire()
>>> w1.is_equal(w2)
False
>>> w1.is_same(w2)
False
>>> pickle.dumps(w1) == pickle.dumps(w2)
True

Comparing the pickle results might not be reliable so I'm not sure this method of comparison should be built into b3d. In general I've found that when something like this is missing from OCCT it's because it's actually quite difficult to do (which hasn't stopped me in the past :-)

To build this feature I guess I would start with building an Edge comparator that extracts the underlying Geom_Curve object and compares all its attributes which would have to be carefully done to account for all possible types - as described by ChatGPT:

Roger> What are the different types of Geom_Curve objects? ChatGPT> Geom_Curve is an abstract class in OpenCascade representing the common interface for various types of curves. Several concrete curve classes inherit from Geom_Curve. Here are some of them:

These are just a few examples, and there might be other specialized curve classes in OpenCascade. Each of these curve types provides specific functionality and properties based on the mathematical characteristics of the curve.

TL;DR: No