mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-10 09:48:32 +00:00
Compare entity instances in one file by identity again
entity_instance.file is a property backed by a fresh SWIG wrapper on every access, and ifcopenshell::file had no __eq__, so `self.file != other.file` in entity_instance.__eq__ compared two throwaway wrappers and was always true - even for an instance against itself. Every entity comparison therefore took the deep get_info() branch, making distinct but structurally identical instances compare equal and leaving the final `return False` unreachable. Bonsai's TestAddRepresentationItemToShapeAspect showed this as two separate IfcShapeAspects being treated as one, so the stale aspect was never removed. Restore the file_pointer() pair that was commented out on both ifcopenshell::file and express::Base - IfcParseWrapper.i already described it as the way to "trace file ownership of instances on the python side" - and give file the __eq__/__hash__ it was missing. The express::Base one needs $self->file() now that file_ lives on instance_data. This also repairs rocksdb_lazy_instance.__eq__, which already called file_pointer(). EXPRESS `=` is value comparison and `:=:` is instance comparison, but rule_compiler emits `==` for both (see the @todo on process_rel_op), and derived attributes build their operands in the shared global file, so rules compare same-file instances and need value semantics. Restore those for the duration of rule execution with settings.compare_instances_by_value, alongside the existing unpack_non_aggregate_inverses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -218,7 +218,11 @@ class entity_instance_mixin:
|
||||
if self.is_a(True) != other.is_a(True):
|
||||
return False
|
||||
|
||||
if self.file != other.file or not self.is_entity():
|
||||
if (
|
||||
settings.compare_instances_by_value
|
||||
or self.file_pointer() != other.file_pointer()
|
||||
or not self.is_entity()
|
||||
):
|
||||
return self.get_info(recursive=True, include_identifier=False) == other.get_info(
|
||||
recursive=True, include_identifier=False
|
||||
)
|
||||
|
||||
@@ -90,6 +90,12 @@ def run(f: ifcopenshell.file, logger: Logger) -> None:
|
||||
orig = ifcopenshell.settings.unpack_non_aggregate_inverses
|
||||
ifcopenshell.settings.unpack_non_aggregate_inverses = True
|
||||
|
||||
# Rules are transpiled with EXPRESS `=` emitted as Python `==`, so `==` has
|
||||
# to mean value comparison for the duration. See the @todo on
|
||||
# rule_compiler.process_rel_op.
|
||||
orig_compare = ifcopenshell.settings.compare_instances_by_value
|
||||
ifcopenshell.settings.compare_instances_by_value = True
|
||||
|
||||
fn = os.path.join(os.path.dirname(__file__), "rules", f"{f.schema_identifier}.py")
|
||||
try:
|
||||
source = open(fn, "r").read()
|
||||
@@ -272,6 +278,7 @@ def run(f: ifcopenshell.file, logger: Logger) -> None:
|
||||
)
|
||||
|
||||
ifcopenshell.settings.unpack_non_aggregate_inverses = orig
|
||||
ifcopenshell.settings.compare_instances_by_value = orig_compare
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
@@ -750,6 +750,7 @@ class entity_instance(entity_instance_mixin):
|
||||
def declaration(self) -> declaration: ...
|
||||
@property
|
||||
def file(self) -> ifcopenshell.file: ...
|
||||
def file_pointer(self) -> int: ...
|
||||
def get_argument(self, *args: int | str) -> Any: ...
|
||||
def get_argument_index(self, a: str) -> int: ...
|
||||
def attribute_name(self, i: int) -> str: ...
|
||||
@@ -836,6 +837,7 @@ class face:
|
||||
def print_impl(self, o, indent): ...
|
||||
|
||||
class file(file_mixin):
|
||||
def file_pointer(self) -> int: ...
|
||||
def fresh_id(self) -> int: ...
|
||||
def __init__(
|
||||
self,
|
||||
|
||||
@@ -34,3 +34,24 @@ element or None. Example:
|
||||
"""
|
||||
|
||||
unpack_non_aggregate_inverses = False
|
||||
|
||||
"""
|
||||
When true compare entity instances by value rather than by identity, even
|
||||
when they belong to the same file. EXPRESS uses `=` for value comparison and
|
||||
`:=:` for instance comparison, whereas the Python API only has `==`, which
|
||||
normally means "the same instance". Example:
|
||||
|
||||
>>> import ifcopenshell
|
||||
>>> f = ifcopenshell.file(schema='ifc2x3')
|
||||
>>> f.createIfcCartesianPoint((0., 0.))
|
||||
#1=IfcCartesianPoint((0.,0.))
|
||||
>>> f.createIfcCartesianPoint((0., 0.))
|
||||
#2=IfcCartesianPoint((0.,0.))
|
||||
>>> f[1] == f[2]
|
||||
False
|
||||
>>> ifcopenshell.settings.compare_instances_by_value = True
|
||||
>>> f[1] == f[2]
|
||||
True
|
||||
"""
|
||||
|
||||
compare_instances_by_value = False
|
||||
|
||||
@@ -75,6 +75,20 @@ def test_equality():
|
||||
assert f[1] == g[1]
|
||||
g[1].Coordinates = (1.0, 0.0)
|
||||
assert f[1] != g[1]
|
||||
f.createIfcCartesianPoint((0.0, 0.0))
|
||||
assert f[1] == f[1]
|
||||
assert f[1] != f[2]
|
||||
|
||||
|
||||
def test_equality_of_owning_file():
|
||||
f = ifcopenshell.file()
|
||||
g = ifcopenshell.file()
|
||||
f.createIfcCartesianPoint((0.0, 0.0))
|
||||
f.createIfcCartesianPoint((0.0, 0.0))
|
||||
g.createIfcCartesianPoint((0.0, 0.0))
|
||||
assert f[1].file == f[1].file
|
||||
assert f[1].file == f[2].file
|
||||
assert f[1].file != g[1].file
|
||||
|
||||
|
||||
def test_setting_logical():
|
||||
|
||||
Reference in New Issue
Block a user