From 611273a20ad265eff1f5f9eaf5c00c8525e648ff Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Wed, 25 Mar 2026 14:57:10 +1100 Subject: [PATCH] Fix add_georeferencing silently failing with orphan CRS or conversion If a file had an IfcProjectedCRS without an IfcCoordinateOperation (or vice versa), add_georeferencing would return early without creating the missing entity. This caused edit_georeferencing to crash with IndexError. Now detects the inconsistent state, cleans up, and recreates both. Co-Authored-By: Claude Opus 4.6 --- .../api/georeference/add_georeferencing.py | 8 ++++++- .../georeference/test_add_georeferencing.py | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/ifcopenshell-python/ifcopenshell/api/georeference/add_georeferencing.py b/src/ifcopenshell-python/ifcopenshell/api/georeference/add_georeferencing.py index 43ccaea2bc..c7e6dfba80 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/georeference/add_georeferencing.py +++ b/src/ifcopenshell-python/ifcopenshell/api/georeference/add_georeferencing.py @@ -17,6 +17,7 @@ # along with IfcOpenShell. If not, see . import ifcopenshell +import ifcopenshell.api.georeference import ifcopenshell.api.pset import ifcopenshell.util.element @@ -63,8 +64,13 @@ def add_georeferencing(file: ifcopenshell.file, ifc_class: str = "IfcMapConversi }, ) return - if file.by_type("IfcProjectedCRS"): + has_crs = bool(file.by_type("IfcProjectedCRS")) + has_conversion = bool(file.by_type("IfcCoordinateOperation")) + if has_crs and has_conversion: return + if has_crs or has_conversion: + # This is technically invalid, but we shall forgive the industry here if they are wrong ... + ifcopenshell.api.georeference.remove_georeferencing(file) source_crs = None for context in file.by_type("IfcGeometricRepresentationContext", include_subtypes=False): if context.ContextType == "Model": diff --git a/src/ifcopenshell-python/test/api/georeference/test_add_georeferencing.py b/src/ifcopenshell-python/test/api/georeference/test_add_georeferencing.py index 68f1ac2597..44e45dec28 100644 --- a/src/ifcopenshell-python/test/api/georeference/test_add_georeferencing.py +++ b/src/ifcopenshell-python/test/api/georeference/test_add_georeferencing.py @@ -51,6 +51,30 @@ class TestAddGeoreferencing(test.bootstrap.IFC4): assert len(self.file.by_type("IfcMapConversion")) == 1 assert len(self.file.by_type("IfcProjectedCRS")) == 1 + def test_recovering_from_orphan_projected_crs(self): + ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject") + ifcopenshell.api.context.add_context(self.file, "Model") + self.file.create_entity("IfcProjectedCRS", Name="EPSG:1234") + assert len(self.file.by_type("IfcProjectedCRS")) == 1 + assert len(self.file.by_type("IfcCoordinateOperation")) == 0 + ifcopenshell.api.georeference.add_georeferencing(self.file) + assert len(self.file.by_type("IfcMapConversion")) == 1 + assert len(self.file.by_type("IfcProjectedCRS")) == 1 + + def test_recovering_from_orphan_coordinate_operation(self): + ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject") + context = ifcopenshell.api.context.add_context(self.file, "Model") + self.file.create_entity("IfcMapConversion", SourceCRS=context, TargetCRS=self.file.create_entity("IfcProjectedCRS", Name="EPSG:1234")) + ifcopenshell.api.georeference.remove_georeferencing(self.file) + # Simulate orphan by re-adding just a conversion without CRS + self.file.create_entity("IfcMapConversion", SourceCRS=context, TargetCRS=self.file.create_entity("IfcProjectedCRS", Name="EPSG:1234")) + self.file.remove(self.file.by_type("IfcProjectedCRS")[0]) + assert len(self.file.by_type("IfcProjectedCRS")) == 0 + assert len(self.file.by_type("IfcCoordinateOperation")) == 1 + ifcopenshell.api.georeference.add_georeferencing(self.file) + assert len(self.file.by_type("IfcMapConversion")) == 1 + assert len(self.file.by_type("IfcProjectedCRS")) == 1 + class TestAddGeoreferencingIFC2X3(test.bootstrap.IFC2X3): def test_adding_georeferencing(self):