From ed17d931a04efd574eb6dec0abe4903dddc683b0 Mon Sep 17 00:00:00 2001 From: lachlangrose Date: Thu, 20 Aug 2026 14:30:37 +0930 Subject: [PATCH 1/3] test: pin training-value/isovalue direction agreement for stratigraphic columns Guards against the swap fixed in 814be12: model_manager.py's per-unit training value and LoopStructural's get_isovalues() must agree on which direction values increase, or extracted isosurfaces get labelled with the wrong unit while keeping correct geometry. Co-Authored-By: Claude Sonnet 5 --- .../test_stratigraphic_value_consistency.py | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 tests/qgis/test_stratigraphic_value_consistency.py diff --git a/tests/qgis/test_stratigraphic_value_consistency.py b/tests/qgis/test_stratigraphic_value_consistency.py new file mode 100644 index 0000000..5135a20 --- /dev/null +++ b/tests/qgis/test_stratigraphic_value_consistency.py @@ -0,0 +1,102 @@ +"""Regression test for the training-value / isovalue direction bug. + +`GeologicalModelManager.update_foliation_features` assigns a scalar `val` to +each unit's basal contact before handing the data to the interpolator. +`StratigraphicColumn.get_isovalues` (LoopStructural core) later decides which +name to stamp on each extracted isosurface, using its own idea of which +value belongs to which unit. + +These two must agree on direction (does value increase from oldest-to- +youngest, or youngest-to-oldest?), or every extracted surface gets labelled +with the wrong unit while keeping correct geometry -- see the "stratigraphic +column was reversed" fixes in model_manager.py (2025-07-21) and the widget +(2025-08-21, reverted 2025-09-08). This has flipped back and forth as this +plugin and LoopStructural evolved independently; this test pins the +invariant so a future change on either side fails loudly here instead of +silently inverting a user's model. +""" + +import pandas as pd +import pytest +from LoopStructural import StratigraphicColumn + +from loopstructural.main.model_manager import GeologicalModelManager + + +def _contact(unit_name): + """A minimal single-point basal contact, tagged with its unit name so + the test can recover which row came from which unit after the group + DataFrames get concatenated.""" + return pd.DataFrame({'X': [0.0], 'Y': [0.0], 'Z': [0.0], 'source_unit': [unit_name]}) + + +@pytest.fixture +def manager(monkeypatch): + manager = GeologicalModelManager() + + captured_calls = [] + + def fake_create_and_add_foliation(name, data=None, **kwargs): + captured_calls.append(data) + return object() # stand-in foliation, only passed back into add_unconformity + + monkeypatch.setattr(manager.model, 'create_and_add_foliation', fake_create_and_add_foliation) + monkeypatch.setattr(manager.model, 'add_unconformity', lambda *a, **k: None) + manager._captured_calls = captured_calls + return manager + + +class TestTrainingValueMatchesIsovalue: + def test_single_group_three_units(self, manager): + column = StratigraphicColumn() + column.clear(basement=False) # single flat group, no unconformities + column.add_unit(name='oldest', thickness=100.0, where='top') + column.add_unit(name='middle', thickness=200.0, where='top') + column.add_unit(name='youngest', thickness=300.0, where='top') + + manager.stratigraphic_column = column + for name in ('oldest', 'middle', 'youngest'): + manager.stratigraphy[name]['contact'] = _contact(name) + + manager.update_foliation_features() + + training_values = self._training_values_by_unit(manager._captured_calls) + expected_values = { + name: entry['value'] for name, entry in column.get_isovalues().items() + } + + for unit_name in ('oldest', 'middle', 'youngest'): + assert training_values[unit_name] == pytest.approx(expected_values[unit_name]), ( + f"'{unit_name}' was trained with val={training_values[unit_name]} but " + f"get_isovalues() will label the value={expected_values[unit_name]} surface " + f"with this unit's name -- the trained field and the isosurface labels " + f"disagree on direction, so extracted surfaces will get the wrong unit name." + ) + + def test_two_groups_split_by_unconformity(self, manager): + column = StratigraphicColumn() + column.clear(basement=False) + column.add_unit(name='basin_floor', thickness=50.0, where='top') + column.add_unit(name='basin_fill', thickness=150.0, where='top') + column.add_unconformity(name='regional_unconformity', where='top') + column.add_unit(name='cover_lower', thickness=80.0, where='top') + column.add_unit(name='cover_upper', thickness=120.0, where='top') + + manager.stratigraphic_column = column + for name in ('basin_floor', 'basin_fill', 'cover_lower', 'cover_upper'): + manager.stratigraphy[name]['contact'] = _contact(name) + + manager.update_foliation_features() + + training_values = self._training_values_by_unit(manager._captured_calls) + expected_values = { + name: entry['value'] for name, entry in column.get_isovalues().items() + } + + for unit_name in ('basin_floor', 'basin_fill', 'cover_lower', 'cover_upper'): + assert training_values[unit_name] == pytest.approx(expected_values[unit_name]) + + @staticmethod + def _training_values_by_unit(captured_calls): + combined = pd.concat(captured_calls, ignore_index=True) + return dict(zip(combined['source_unit'], combined['val'])) From c0d687550e2bec2e556e9d25eeecdddca655d0ff Mon Sep 17 00:00:00 2001 From: lachlangrose Date: Mon, 24 Aug 2026 12:52:22 +0930 Subject: [PATCH 2/3] fix: connect layer change to update field combo boxes --- loopstructural/gui/map2loop_tools/sorter_widget.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/loopstructural/gui/map2loop_tools/sorter_widget.py b/loopstructural/gui/map2loop_tools/sorter_widget.py index 298ea6b..8e53e02 100644 --- a/loopstructural/gui/map2loop_tools/sorter_widget.py +++ b/loopstructural/gui/map2loop_tools/sorter_widget.py @@ -70,6 +70,7 @@ def __init__(self, parent=None, data_manager=None, debug_manager=None): self.sortingAlgorithmComboBox.currentIndexChanged.connect(self._on_algorithm_changed) self.geologyLayerComboBox.layerChanged.connect(self._on_geology_layer_changed) self.structureLayerComboBox.layerChanged.connect(self._on_structure_layer_changed) + self.contactsLayerComboBox.layerChanged.connect(self._on_contacts_layer_changed) self.runButton.clicked.connect(self._run_sorter) self.orientationTypeComboBox.setCurrentIndex(1) # Default to Dip Direction self._guess_layers() @@ -278,6 +279,12 @@ def _on_structure_layer_changed(self): if dipdir_match := matcher.find_match('DIPDIR'): self.dipDirFieldComboBox.setField(dipdir_match) + def _on_contacts_layer_changed(self): + """Update field combo boxes when contacts layer changes.""" + layer = self.contactsLayerComboBox.currentLayer() + self.unitName1FieldComboBox.setLayer(layer) + self.unitName2FieldComboBox.setLayer(layer) + def _on_algorithm_changed(self): """Update UI based on selected sorting algorithm and map2loop requirements.""" algorithm_index = self.sortingAlgorithmComboBox.currentIndex() From 0516d7169550ce8ad22a2ff85de80caf32c5678d Mon Sep 17 00:00:00 2001 From: lachlangrose Date: Mon, 24 Aug 2026 13:19:36 +0930 Subject: [PATCH 3/3] style: fix black formatting and ruff lint (unnecessary generator) Co-Authored-By: Claude Sonnet 5 --- .../gui/map2loop_tools/fault_topology_widget.py | 6 ++---- .../gui/modelling/model_definition/bounding_box.py | 8 ++++---- .../gui/visualisation/feature_list_widget.py | 11 +++++++---- loopstructural/main/m2l_api.py | 4 +++- tests/qgis/test_stratigraphic_value_consistency.py | 8 ++------ 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/loopstructural/gui/map2loop_tools/fault_topology_widget.py b/loopstructural/gui/map2loop_tools/fault_topology_widget.py index 5d786ac..76fa7ff 100644 --- a/loopstructural/gui/map2loop_tools/fault_topology_widget.py +++ b/loopstructural/gui/map2loop_tools/fault_topology_widget.py @@ -174,7 +174,7 @@ def _run_topology(self): # not just the ones map2loop found a relationship for. A fault with # no detected topological relationship is still a real fault and # must not be dropped from the fault topology. - new_faults = set(str(v) for v in gdf['ID'].unique()) + new_faults = {str(v) for v in gdf['ID'].unique()} # Add new faults; never remove existing ones here, so faults # without a detected relationship (or ones the user added @@ -209,9 +209,7 @@ def _run_topology(self): else: f1 = str(row.iloc[0]) f2 = str(row.iloc[1]) - ft.update_fault_relationship( - f1, f2, FaultRelationshipType.ABUTTING - ) + ft.update_fault_relationship(f1, f2, FaultRelationshipType.ABUTTING) except Exception: pass diff --git a/loopstructural/gui/modelling/model_definition/bounding_box.py b/loopstructural/gui/modelling/model_definition/bounding_box.py index d5062f4..4c594f6 100644 --- a/loopstructural/gui/modelling/model_definition/bounding_box.py +++ b/loopstructural/gui/modelling/model_definition/bounding_box.py @@ -32,11 +32,11 @@ def __init__(self, parent=None, data_manager=None): self.selectFromCurrentLayerButton, "mActionZoomToLayer.svg", "Select from Current Layer" ) self._style_tool_button( - self.useCurrentViewExtentButton, "mActionSetToCanvasExtent.svg", "Use Current View Extent" - ) - self._style_tool_button( - self.drawOnMapButton, "mActionAddBasicRectangle.svg", "Draw on Map" + self.useCurrentViewExtentButton, + "mActionSetToCanvasExtent.svg", + "Use Current View Extent", ) + self._style_tool_button(self.drawOnMapButton, "mActionAddBasicRectangle.svg", "Draw on Map") self.drawOnMapButton.setCheckable(True) self.drawOnMapButton.clicked.connect(self.drawOnMap) self._draw_extent_tool = None diff --git a/loopstructural/gui/visualisation/feature_list_widget.py b/loopstructural/gui/visualisation/feature_list_widget.py index e6e6539..90c17a2 100644 --- a/loopstructural/gui/visualisation/feature_list_widget.py +++ b/loopstructural/gui/visualisation/feature_list_widget.py @@ -56,9 +56,7 @@ def __init__(self, parent=None, *, model_manager=None, viewer=None, data_manager self.data_manager = data_manager # Add buttons - self.addBoundingBoxButton = self._make_tool_button( - "extents.svg", "Add Model Bounding Box" - ) + self.addBoundingBoxButton = self._make_tool_button("extents.svg", "Add Model Bounding Box") self.addFaultSurfacesButton = self._make_custom_icon_tool_button( "fault.svg", "Add Fault Surfaces" ) @@ -724,7 +722,12 @@ def _extract_line_xy(self, layer) -> Optional[np.ndarray]: except Exception: target_crs = None source_crs = layer.sourceCrs() - if target_crs is not None and target_crs.isValid() and source_crs.isValid() and source_crs != target_crs: + if ( + target_crs is not None + and target_crs.isValid() + and source_crs.isValid() + and source_crs != target_crs + ): geom = QgsGeometry(geom) geom.transform(QgsCoordinateTransform(source_crs, target_crs, QgsProject.instance())) diff --git a/loopstructural/main/m2l_api.py b/loopstructural/main/m2l_api.py index 2b5f1d8..82aa0a8 100644 --- a/loopstructural/main/m2l_api.py +++ b/loopstructural/main/m2l_api.py @@ -98,7 +98,9 @@ def extract_basal_contacts( unit_name_col = 'UNITNAME' if 'UNITNAME' in geology.columns else unit_name_field if unit_name_col and unit_name_col in geology.columns: geology_unit_names = {str(v).strip() for v in geology[unit_name_col].dropna().unique()} - stratigraphic_names = {str(name).strip() for name in stratigraphic_order if name is not None} + stratigraphic_names = { + str(name).strip() for name in stratigraphic_order if name is not None + } ignored_names = {str(unit).strip() for unit in ignore_units if unit is not None} missing_from_column = sorted(geology_unit_names - stratigraphic_names - ignored_names) if missing_from_column: diff --git a/tests/qgis/test_stratigraphic_value_consistency.py b/tests/qgis/test_stratigraphic_value_consistency.py index 5135a20..583fa53 100644 --- a/tests/qgis/test_stratigraphic_value_consistency.py +++ b/tests/qgis/test_stratigraphic_value_consistency.py @@ -61,9 +61,7 @@ def test_single_group_three_units(self, manager): manager.update_foliation_features() training_values = self._training_values_by_unit(manager._captured_calls) - expected_values = { - name: entry['value'] for name, entry in column.get_isovalues().items() - } + expected_values = {name: entry['value'] for name, entry in column.get_isovalues().items()} for unit_name in ('oldest', 'middle', 'youngest'): assert training_values[unit_name] == pytest.approx(expected_values[unit_name]), ( @@ -89,9 +87,7 @@ def test_two_groups_split_by_unconformity(self, manager): manager.update_foliation_features() training_values = self._training_values_by_unit(manager._captured_calls) - expected_values = { - name: entry['value'] for name, entry in column.get_isovalues().items() - } + expected_values = {name: entry['value'] for name, entry in column.get_isovalues().items()} for unit_name in ('basin_floor', 'basin_fill', 'cover_lower', 'cover_upper'): assert training_values[unit_name] == pytest.approx(expected_values[unit_name])