CadQuery / CadQuery/OCP

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

Open
#161 3 comments 1 reaction 0 assignees View on GitHub
Dominant language
C++
Stars
197
Forks
47
Avg merge
2d 2h
Merged PRs (30d)
6

Description

`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:
```py
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
```py
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:
```py
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
```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.