From b3dc0b447856b5d3781858d8f89c0a6345e1283e Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Thu, 9 Jul 2026 13:24:31 +0300 Subject: [PATCH] fix(ifcdiff): check attributes by default so PredefinedType changes are caught (#8214) IfcDiff defaulted to relationships=["geometry"], so a plain diff only ever compared geometry. Attribute-only edits on an element that kept its GlobalId (a modified or removed PredefinedType, a renamed element, etc.) were silently missed. The CLI made this worse: --relationships did not list "attributes" or "geometry" as valid values, so there was no documented way to enable it. The default is now ["attributes", "geometry"], so a plain `ifcdiff old new` reports attribute changes alongside geometry changes. The CLI help and the IfcDiff docstring now document all valid relationship values. Added a regression test covering a PredefinedType change detected with the default configuration. Co-Authored-By: Claude Sonnet 4.6 (cherry picked from commit 0a8ae147898fdd1696b89f321d3df6635c7363b1) --- src/ifcdiff/ifcdiff.py | 14 ++++++++++---- src/ifcdiff/test.py | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/ifcdiff/ifcdiff.py b/src/ifcdiff/ifcdiff.py index 3032dd23ce..b7ff76daed 100755 --- a/src/ifcdiff/ifcdiff.py +++ b/src/ifcdiff/ifcdiff.py @@ -51,8 +51,10 @@ class IfcDiff: :param old: IFC file object for the old model :param new: IFC file object for the new model - :param relationships: List of relationships to check. None means that only - geometry is compared. See RELATIONSHIP_TYPE for available relationships. + :param relationships: List of relationships to check. None means that + attributes and geometry are compared, so changes such as a modified or + removed PredefinedType are reported. See RELATIONSHIP_TYPE for available + relationships. :param is_shallow: True if you want only the first difference to be listed. False if you want all differences to be checked. Choosing False means that comparisons will take longer. @@ -86,7 +88,7 @@ class IfcDiff: self.new = new self.change_register = {} self.representation_ids = {} - self.relationships = relationships or ["geometry"] + self.relationships = relationships or ["attributes", "geometry"] self.precision = 1e-4 self.is_shallow = is_shallow self.filter_elements = filter_elements @@ -435,7 +437,11 @@ if __name__ == "__main__": "-r", "--relationships", type=str, - help='A list of space-separated relationships, chosen from "type", "property", "container", "aggregate", "classification"', + help=( + 'A list of space-separated relationships, chosen from "attributes", "geometry", ' + '"type", "property", "container", "aggregate", "classification". ' + 'Defaults to "attributes geometry" when omitted.' + ), default="", ) args = parser.parse_args() diff --git a/src/ifcdiff/test.py b/src/ifcdiff/test.py index ca294a1ed4..b661c0f70b 100644 --- a/src/ifcdiff/test.py +++ b/src/ifcdiff/test.py @@ -77,6 +77,23 @@ class TestIfcDiff: assert ifc_diff.deleted_elements == set() assert ifc_diff.change_register == {wall.GlobalId: {"attributes_changed": True}} + def test_changed_predefined_type_is_caught_by_default(self): + # Regression test for #8214: a plain diff (no relationships specified) + # must report a modified or removed PredefinedType. Previously the + # default only compared geometry, so attribute-only edits were missed. + ifc_file = setup_project() + wall = ifcopenshell.api.root.create_entity(ifc_file, ifc_class="IfcWall", name="Foo") + wall.PredefinedType = "SOLIDWALL" + + new_file = ifc_file.from_string(ifc_file.to_string()) + new_file.by_id(wall.id()).PredefinedType = "NOTDEFINED" + + ifc_diff = ifcdiff.IfcDiff(ifc_file, new_file) + ifc_diff.diff() + assert ifc_diff.added_elements == set() + assert ifc_diff.deleted_elements == set() + assert ifc_diff.change_register == {wall.GlobalId: {"attributes_changed": True}} + def test_changed_geometry(self): ifc_file = setup_project() wall = ifcopenshell.api.root.create_entity(ifc_file, ifc_class="IfcWall", name="Foo")