fixed sample reordering for super/subphase #329

EasyReflectometryApp/Backends/Py/logic/layers.py

@@ -40,18 +40,6 @@ def _sample(self) -> Sample:
     def _layers(self) -> LayerCollection:
         return self._sample[self._project_lib.current_assembly_index].layers
 
-    @property
-    def _assembly_type(self) -> str:
-        """Determine if current assembly is superphase, subphase, or regular."""
-        current_index = self._project_lib.current_assembly_index
-        total_assemblies = len(self._sample)
-        if current_index == 0:
-            return 'superphase'
-        elif current_index == total_assemblies - 1:
-            return 'subphase'
-        else:
-            return 'regular'
-
     @property
     def index(self) -> int:
         return self._project_lib.current_layer_index
@@ -66,7 +54,7 @@ def name_at_current_index(self) -> str:
 
     @property
     def layers(self) -> list[dict[str, str]]:
-        return _from_layers_collection_to_list_of_dicts(self._layers, self._assembly_type)
+        return _from_layers_collection_to_list_of_dicts(self._layers)
 
     @property
     def layers_names(self) -> list[str]:
@@ -393,28 +381,18 @@ def _set_parameter_value(parameter, new_value) -> bool:
     return True
 
 
-def _from_layers_collection_to_list_of_dicts(
-    layers_collection: LayerCollection, assembly_type: str = 'regular'
-) -> list[dict[str, str]]:
+def _from_layers_collection_to_list_of_dicts(layers_collection: LayerCollection) -> list[dict[str, str]]:
     """Convert layers collection to list of dicts.
 
+    Editability of thickness and roughness mirrors each parameter's ``enabled``
+    flag, which ``Project._update_enablement_of_fixed_layers_for_model`` keeps
+    in step with the superphase (first layer of the first assembly) and the
+    subphase (last layer of the last assembly). Reading the flag rather than
+    re-deriving it from the assembly position keeps the table and the
+    parameters in agreement (issue #329).
+
     :param layers_collection: The collection of layers.
-    :param assembly_type: Type of assembly - 'superphase', 'subphase', or 'regular'.
-        - superphase: Neither thickness nor roughness should be editable
-        - subphase: Only roughness should be editable
-        - regular: Both thickness and roughness should be editable
     """
-    # Determine enabled states based on assembly type
-    if assembly_type == 'superphase':
-        thickness_enabled = 'False'
-        roughness_enabled = 'False'
-    elif assembly_type == 'subphase':
-        thickness_enabled = 'False'
-        roughness_enabled = 'True'
-    else:  # regular
-        thickness_enabled = 'True'
-        roughness_enabled = 'True'
-
     layers_list = []
     for layer in layers_collection:
         layers_list.append(
@@ -428,8 +406,10 @@ def _from_layers_collection_to_list_of_dicts(
                 'solvent': 'solvent',
                 'solvation': '0.2',
                 'apm_enabled': 'True',
-                'thickness_enabled': thickness_enabled,
-                'roughness_enabled': roughness_enabled,
+                # `enabled` is set on a parameter only once enablement has run
+                # over it; until then it counts as enabled, as in the library.
+                'thickness_enabled': str(bool(getattr(layer.thickness, 'enabled', True))),
+                'roughness_enabled': str(bool(getattr(layer.roughness, 'enabled', True))),
             }
         )
         if isinstance(layer, LayerAreaPerMolecule):

EasyReflectometryApp/Backends/Py/logic/project.py

@@ -108,9 +108,11 @@ def experimental_data_at_current_index(self) -> bool:
     def _update_enablement_of_fixed_layers_for_model(self, index: int) -> None:
         """Re-derive superphase/subphase layer enablement from assembly position.
 
-        Enablement is position-based (superphase = first assembly, subphase =
-        last), so it must be recomputed whenever assemblies are added, removed,
-        or reordered. This method is idempotent: it first re-enables every layer,
+        Enablement is position-based (superphase = first layer of the first
+        assembly, subphase = last layer of the last assembly), so it must be
+        recomputed whenever assemblies, or the layers inside them, are added,
+        removed, duplicated or reordered. The layer table reads the resulting
+        flags. This method is idempotent: it first re-enables every layer,
         then disables the fixed superphase/subphase parameters. A full reset is
         safe because this is the only code path that disables layer enablement
         (issue #329).

EasyReflectometryApp/Backends/Py/sample.py

@@ -774,6 +774,7 @@ def setLayerSolvationAtIndex(self, index: int, new_value: float) -> None:
     @Slot(str)
     def removeLayer(self, value: str) -> None:
         self._layers_logic.remove_at_index(value)
+        self._project_logic._update_enablement_of_fixed_layers_for_model(self._models_logic.index)
         self._detach_unreachable_constraints()
         self._clearCacheAndEmitLayersChanged()
         self.externalRefreshPlot.emit()
@@ -782,26 +783,30 @@ def removeLayer(self, value: str) -> None:
     @Slot()
     def addNewLayer(self) -> None:
         self._layers_logic.add_new()
+        self._project_logic._update_enablement_of_fixed_layers_for_model(self._models_logic.index)
         self._clearCacheAndEmitLayersChanged()
         self.externalRefreshPlot.emit()
         self.externalSampleChanged.emit()
 
     @Slot()
     def duplicateSelectedLayer(self) -> None:
         self._layers_logic.duplicate_selected()
+        self._project_logic._update_enablement_of_fixed_layers_for_model(self._models_logic.index)
         self._clearCacheAndEmitLayersChanged()
         self.externalRefreshPlot.emit()
         self.externalSampleChanged.emit()
 
     @Slot()
     def moveSelectedLayerUp(self) -> None:
         self._layers_logic.move_selected_up()
+        self._project_logic._update_enablement_of_fixed_layers_for_model(self._models_logic.index)
         self._clearCacheAndEmitLayersChanged()
         self.externalRefreshPlot.emit()
 
     @Slot()
     def moveSelectedLayerDown(self) -> None:
         self._layers_logic.move_selected_down()
+        self._project_logic._update_enablement_of_fixed_layers_for_model(self._models_logic.index)
         self._clearCacheAndEmitLayersChanged()
         self.externalRefreshPlot.emit()
 

tests/test_logic_layers.py

@@ -12,7 +12,7 @@
 from tests.factories import make_sample
 
 
-def test_from_layers_collection_to_list_of_dicts_respects_assembly_enablement(monkeypatch):
+def test_from_layers_collection_to_list_of_dicts_mirrors_parameter_enablement(monkeypatch):
     monkeypatch.setattr(layers_module, 'LayerAreaPerMolecule', FakeLayerAreaPerMolecule)
     layer = FakeLayerAreaPerMolecule(
         name='Headgroup',
@@ -25,18 +25,22 @@ def test_from_layers_collection_to_list_of_dicts_respects_assembly_enablement(mo
         molecular_formula='C12H25',
     )
 
-    regular = layers_module._from_layers_collection_to_list_of_dicts(make_layer_collection(layer), 'regular')
-    superphase = layers_module._from_layers_collection_to_list_of_dicts(make_layer_collection(layer), 'superphase')
-    subphase = layers_module._from_layers_collection_to_list_of_dicts(make_layer_collection(layer), 'subphase')
-
+    regular = layers_module._from_layers_collection_to_list_of_dicts(make_layer_collection(layer))
     assert regular[0]['thickness_enabled'] == 'True'
     assert regular[0]['roughness_enabled'] == 'True'
     assert regular[0]['formula'] == 'C12H25'
     assert regular[0]['apm'] == '44.0'
     assert regular[0]['solvent'] == 'Water'
     assert regular[0]['solvation'] == '0.35'
+
+    layer.thickness.enabled = False
+    layer.roughness.enabled = False
+    superphase = layers_module._from_layers_collection_to_list_of_dicts(make_layer_collection(layer))
     assert superphase[0]['thickness_enabled'] == 'False'
     assert superphase[0]['roughness_enabled'] == 'False'
+
+    layer.roughness.enabled = True
+    subphase = layers_module._from_layers_collection_to_list_of_dicts(make_layer_collection(layer))
     assert subphase[0]['thickness_enabled'] == 'False'
     assert subphase[0]['roughness_enabled'] == 'True'
 
@@ -81,7 +85,6 @@ def test_layers_move_duplicate_and_setters_update_current_layer(monkeypatch):
     project.current_layer_index = 1
     logic = layers_module.Layers(project)
 
-    assert logic._assembly_type == 'regular'
     assert logic.set_name_at_current_index('Renamed') is True
     assert logic.set_thickness_at_current_index(25.0) is True
     assert logic.set_roughness_at_current_index(5.0) is True

tests/test_py_sample_fixed_layers.py

@@ -0,0 +1,163 @@
+"""Superphase/subphase enablement under layer edits (issue #329).
+
+The superphase is the first layer of the first assembly and the subphase the
+last layer of the last assembly, so editing the layers inside those assemblies
+moves which parameters are fixed. These tests use the real reflectometry
+library because enablement lives on its parameters.
+"""
+
+import pytest
+from easyreflectometry import Project
+from easyscience import global_object
+
+from EasyReflectometryApp.Backends.Py.sample import Sample
+
+
+@pytest.fixture(autouse=True)
+def clear_global_map():
+    global_object.map._clear()
+    yield
+    global_object.map._clear()
+
+
+@pytest.fixture
+def project_and_backend(qcore_application):
+    project = Project()
+    backend = Sample(project)  # installs the default model
+    return project, backend
+
+
+def _enabled(parameter):
+    # The attribute appears only once enablement has run over the parameter.
+    return getattr(parameter, 'enabled', True)
+
+
+def _flags(layer):
+    return _enabled(layer.thickness), _enabled(layer.roughness)
+
+
+def _assert_only_fixed_layers_disabled(project):
+    sample = project.models[0].sample
+    for a, assembly in enumerate(sample):
+        for i, layer in enumerate(assembly.layers):
+            is_superphase = a == 0 and i == 0
+            is_subphase = a == len(sample) - 1 and i == len(assembly.layers) - 1
+            assert _enabled(layer.thickness) is not (is_superphase or is_subphase), (a, i)
+            assert _enabled(layer.roughness) is not is_superphase, (a, i)
+
+
+def _assert_table_matches_parameters(project, backend):
+    layers = project.models[0].sample[project.current_assembly_index].layers
+    rows = backend.layers
+    assert [row['thickness_enabled'] for row in rows] == [str(_enabled(layer.thickness)) for layer in layers]
+    assert [row['roughness_enabled'] for row in rows] == [str(_enabled(layer.roughness)) for layer in layers]
+
+
+def test_table_treats_a_parameter_without_enablement_as_editable(project_and_backend):
+    project, backend = project_and_backend
+    layer = project.models[0].sample[1].layers[0]
+    del layer.thickness.enabled
+    backend.setCurrentAssemblyIndex(1)
+    backend.layersConnectChanges()
+
+    assert backend.layers[0]['thickness_enabled'] == 'True'
+
+
+class TestSuperphaseLayerEdits:
+    def test_added_layer_moved_to_top_becomes_the_superphase(self, project_and_backend):
+        project, backend = project_and_backend
+        backend.setCurrentAssemblyIndex(0)
+        original = project.models[0].sample[0].layers[0]
+
+        backend.addNewLayer()
+        added = project.models[0].sample[0].layers[1]
+        _assert_only_fixed_layers_disabled(project)
+        assert _flags(added) == (True, True)
+
+        backend.setCurrentLayerIndex(1)
+        backend.moveSelectedLayerUp()
+
+        assert project.models[0].sample[0].layers[0] is added
+        assert _flags(added) == (False, False)
+        assert _flags(original) == (True, True)
+        _assert_only_fixed_layers_disabled(project)
+        _assert_table_matches_parameters(project, backend)
+
+    def test_move_down_hands_the_superphase_to_the_next_layer(self, project_and_backend):
+        project, backend = project_and_backend
+        backend.setCurrentAssemblyIndex(0)
+        backend.addNewLayer()
+        original = project.models[0].sample[0].layers[0]
+
+        backend.setCurrentLayerIndex(0)
+        backend.moveSelectedLayerDown()
+
+        assert _flags(original) == (True, True)
+        _assert_only_fixed_layers_disabled(project)
+        _assert_table_matches_parameters(project, backend)
+
+    def test_duplicated_superphase_copy_is_editable(self, project_and_backend):
+        project, backend = project_and_backend
+        backend.setCurrentAssemblyIndex(0)
+        backend.setCurrentLayerIndex(0)
+
+        backend.duplicateSelectedLayer()
+
+        assert len(project.models[0].sample[0].layers) == 2
+        _assert_only_fixed_layers_disabled(project)
+        _assert_table_matches_parameters(project, backend)
+
+    def test_removing_the_superphase_layer_fixes_its_successor(self, project_and_backend):
+        project, backend = project_and_backend
+        backend.setCurrentAssemblyIndex(0)
+        backend.addNewLayer()
+        successor = project.models[0].sample[0].layers[1]
+
+        backend.removeLayer('0')
+
+        assert project.models[0].sample[0].layers[0] is successor
+        assert _flags(successor) == (False, False)
+        _assert_only_fixed_layers_disabled(project)
+        _assert_table_matches_parameters(project, backend)
+
+
+class TestSubphaseLayerEdits:
+    def test_added_layer_becomes_the_subphase(self, project_and_backend):
+        project, backend = project_and_backend
+        last = len(project.models[0].sample) - 1
+        backend.setCurrentAssemblyIndex(last)
+        original = project.models[0].sample[last].layers[-1]
+
+        backend.addNewLayer()
+
+        added = project.models[0].sample[last].layers[-1]
+        assert added is not original
+        assert _flags(added) == (False, True)
+        assert _flags(original) == (True, True)
+        _assert_only_fixed_layers_disabled(project)
+        _assert_table_matches_parameters(project, backend)
+
+    def test_move_up_hands_the_subphase_back(self, project_and_backend):
+        project, backend = project_and_backend
+        last = len(project.models[0].sample) - 1
+        backend.setCurrentAssemblyIndex(last)
+        original = project.models[0].sample[last].layers[-1]
+        backend.addNewLayer()
+
+        backend.setCurrentLayerIndex(1)
+        backend.moveSelectedLayerUp()
+
+        assert project.models[0].sample[last].layers[-1] is original
+        assert _flags(original) == (False, True)
+        _assert_only_fixed_layers_disabled(project)
+        _assert_table_matches_parameters(project, backend)
+
+
+def test_middle_assembly_table_is_fully_editable(project_and_backend):
+    project, backend = project_and_backend
+    backend.setCurrentAssemblyIndex(1)
+    backend.addNewLayer()
+
+    assert all(row['thickness_enabled'] == 'True' for row in backend.layers)
+    assert all(row['roughness_enabled'] == 'True' for row in backend.layers)
+    _assert_only_fixed_layers_disabled(project)