Should `TopoDS_Shape` implement `IsEqual` as `__eq__`?

[snoyer]

TopoDS_Shape used to have a .HashCode() method and it now uses the builtin hash() (via .__hash__()). However it still uses a .IsEqual() method and does not implement == (via .__eq__()).

This leads to counter-intuitive behavior such as:

print(f"{[hash(a)==hash(b) for a,b in zip(edges1,edges2)] = }")  # [True, True, True, True, True, True]
print(f"{[a.IsEqual(b) for a,b in zip(edges1,edges2)] = }")  # [True, True, True, True, True, True]
# but...
print(f"{len(set(edges2) - set(edges1)) = }")  # 6    <- would expect 0 as `IsEqual()` and `hash()` suggest the faces are the same
print(f"{[a==b for a,b in zip(edges1,edges2)] = }")  # [False, False, False, False, False, False]  <- that's why :(

if __eq__ was an alias of IsEqual (as can be simulated with TopoDS_Shape.__eq__ = TopoDS_Shape.IsEqual) we'd have

print(f"{[a==b for a,b in zip(edges1,edges2)] = }")  # [True, True, True, True, True, True]
print(f"{len(set(edges2) - set(edges1)) = }")  # 0  <- exclude the faces form themselves, nothing left :)

Are there any reasons not to do this? (arguably __bool__ for .IsNull() would be nice too, but that doesn't seem as important as __hash__ and __eq__ being consistent)

full example:

from typing import Iterator

from OCP.BRepPrimAPI import BRepPrimAPI_MakeBox
from OCP.gp import gp_Ax2
from OCP.TopAbs import TopAbs_ShapeEnum
from OCP.TopExp import TopExp_Explorer
from OCP.TopoDS import TopoDS_Shape


def topexp_explorer(
    shape: TopoDS_Shape, find: TopAbs_ShapeEnum
) -> Iterator[TopoDS_Shape]:
    explorer = TopExp_Explorer(shape, ToFind=find)
    while explorer.More():
        item = explorer.Current()
        yield item
        explorer.Next()


box = BRepPrimAPI_MakeBox(gp_Ax2(), 3, 2, 1).Solid()
edges1 = list(topexp_explorer(box, TopAbs_ShapeEnum.TopAbs_FACE))
edges2 = list(topexp_explorer(box, TopAbs_ShapeEnum.TopAbs_FACE))


print(f"{len(edges1) = }")  # 6
print(f"{len(edges2) = }")  # 6
print(f"{[hash(a)==hash(b) for a,b in zip(edges1,edges2)] = }")  # [True, True, True, True, True, True]
print(f"{[a.IsEqual(b) for a,b in zip(edges1,edges2)] = }")  # [True, True, True, True, True, True]
print(f"{[a==b for a,b in zip(edges1,edges2)] = }")  # [False, False, False, False, False, False]
print(f"{len(set(edges2) - set(edges1)) = }")  # 6


TopoDS_Shape.__eq__ = TopoDS_Shape.IsEqual
print(f"{[a==b for a,b in zip(edges1,edges2)] = }")  # [True, True, True, True, True, True]
print(f"{len(set(edges2) - set(edges1)) = }")  # 0

[snoyer]

arguably bool for .IsNull() would be nice too

Actually it looks like that is already the case in 7.8.1, does that constitute another argument in favor of using __eq__?

[adam-urbanczyk]

For now not planned, but I'll leave this open

[snoyer]

hey @adam-urbanczyk thanks for the reply, do you have any further input as the maintainer of the project?

For now not planned, but I'll leave this open

What other elements could help decide one way or another?

To reiterate in a more concise way in case providing a full argument and example code wasn't the right thing to do:

Two of the TopoDS_Shape methods have been "moved" to Python's builtin (HashCode() to __hash__ and IsNull to __bool__) but IsEqual, which could follow as __eq__, remains.

Is there a specific reason for that?