From cc5cc182c3ff36ac5e3c6a6eec2607965d4a6564 Mon Sep 17 00:00:00 2001 From: rozyczko Date: Thu, 29 Oct 2026 15:11:08 +0100 Subject: [PATCH 1/2] fixed from #311, issue 2.1 --- EasyReflectometryApp/Backends/Mock/Sample.qml | 15 ++--- EasyReflectometryApp/Backends/Py/analysis.py | 26 +++----- .../Backends/Py/logic/assemblies.py | 21 +------ .../Backends/Py/logic/bayesian.py | 6 +- .../Backends/Py/logic/layers.py | 23 ++++--- .../Backends/Py/logic/minimizers.py | 21 ++++--- .../Backends/Py/logic/parameters.py | 59 ++++++++---------- .../Backends/Py/logic/project.py | 2 + EasyReflectometryApp/Backends/Py/project.py | 2 +- EasyReflectometryApp/Backends/Py/sample.py | 28 ++++----- .../Gui/Globals/BackendWrapper.qml | 2 +- .../Sidebar/Advanced/Groups/Constraints.qml | 2 +- .../Basic/Groups/Assemblies/MultiLayer.qml | 4 +- .../Groups/Assemblies/SurfactantLayer.qml | 5 +- tests/test_analysis_bayesian.py | 36 ++++++----- tests/test_logic_assemblies.py | 16 ++--- tests/test_logic_layers.py | 25 +++++--- tests/test_logic_parameters.py | 60 ++++++++++++++++--- tests/test_py_sample_constraints.py | 8 +-- tests/test_py_sample_fixed_layers.py | 25 ++++++-- 20 files changed, 200 insertions(+), 186 deletions(-) diff --git a/EasyReflectometryApp/Backends/Mock/Sample.qml b/EasyReflectometryApp/Backends/Mock/Sample.qml index 43d86575..0efd17e2 100644 --- a/EasyReflectometryApp/Backends/Mock/Sample.qml +++ b/EasyReflectometryApp/Backends/Mock/Sample.qml @@ -226,36 +226,33 @@ QtObject { 'material': 'label 1', 'solvent': 'label 1', 'thickness': '1', - 'thickness_enabled': 'True', + 'thickness_enabled': true, 'roughness': '1', - 'roughness_enabled': 'True', + 'roughness_enabled': true, 'solvation': '1', 'apm': '1', - 'apm_enabled': 'True', }, { 'formula': 'formula 2', 'material': 'label 2', 'solvent': 'label 2', 'thickness': ' 2', - 'thickness_enabled': 'False', + 'thickness_enabled': false, 'roughness': '2', - 'roughness_enabled': 'False', + 'roughness_enabled': false, 'solvation': '2', 'apm': '2', - 'apm_enabled': 'False', }, { 'formula': 'formula 3', 'material': 'label 3', 'solvent': 'label 3', 'thickness': '3', - 'thickness_enabled': 'False', + 'thickness_enabled': false, 'roughness': '3', - 'roughness_enabled': 'False', + 'roughness_enabled': false, 'solvation': '3', 'apm': '3', - 'apm_enabled': 'False', }, ] diff --git a/EasyReflectometryApp/Backends/Py/analysis.py b/EasyReflectometryApp/Backends/Py/analysis.py index a14547db..06f3debe 100644 --- a/EasyReflectometryApp/Backends/Py/analysis.py +++ b/EasyReflectometryApp/Backends/Py/analysis.py @@ -13,6 +13,7 @@ from PySide6.QtCore import Signal from PySide6.QtCore import Slot +from .logic.bayesian import VALID_INITIALIZERS from .logic.bayesian import Bayesian as BayesianLogic from .logic.calculators import Calculators as CalculatorsLogic from .logic.experiment_selection import ExperimentSelection @@ -276,7 +277,7 @@ def setBayesianInitializer(self, value: str) -> None: @Property('QVariantList', notify=minimizerChanged) def bayesianInitializerOptions(self) -> list: - return ['eps', 'cov', 'lhs', 'random'] + return list(VALID_INITIALIZERS) @Property(bool, notify=fittingChanged) def bayesianResultAvailable(self) -> bool: @@ -351,29 +352,17 @@ def _bayesian_display_name_list(self) -> list[str]: @Property(str, notify=fittingChanged) def bayesianCornerPlotUrl(self) -> str: - """Return a file URL for the corner plot PNG, or empty string.""" - if not self._bayesian_logic.has_result: - return '' - if not self._bayesian_logic.corner_plot_url: - self._render_corner_plot() + """File URL of the corner plot HTML, or ''; rendered when sampling finishes, never on read.""" return self._bayesian_logic.corner_plot_url @Property(str, notify=fittingChanged) def bayesianTracePlotUrl(self) -> str: - """Return a file URL for the trace plot PNG, or empty string.""" - if not self._bayesian_logic.has_result: - return '' - if not self._bayesian_logic.trace_plot_url: - self._render_trace_plot() + """File URL of the trace plot HTML, or ''; rendered when sampling finishes, never on read.""" return self._bayesian_logic.trace_plot_url @Property(str, notify=fittingChanged) def bayesianDistributionPlotUrl(self) -> str: - """Return a file URL for the interactive distribution HTML, or empty string.""" - if not self._bayesian_logic.has_result: - return '' - if not self._bayesian_logic.distribution_plot_url: - self._render_distribution_plot() + """File URL of the distribution plot HTML, or ''; rendered when sampling finishes, never on read.""" return self._bayesian_logic.distribution_plot_url @Property('QVariantMap', notify=fittingChanged) @@ -659,6 +648,7 @@ def _on_sample_finished(self, results: list) -> None: self._compute_diagnostics, self._render_corner_plot, self._render_trace_plot, + self._render_distribution_plot, ): try: step() @@ -1384,7 +1374,7 @@ def modelParametersCount(self) -> int: [ parameter for parameter in self._parameters_logic.all_parameters() - if parameter.get('enabled', True) and not self._parameters_logic.is_experiment_parameter(parameter) + if parameter.get('enabled', True) and not parameter['experiment'] ] ) @@ -1394,7 +1384,7 @@ def experimentParametersCount(self) -> int: [ parameter for parameter in self._parameters_logic.all_parameters() - if parameter.get('enabled', True) and self._parameters_logic.is_experiment_parameter(parameter) + if parameter.get('enabled', True) and parameter['experiment'] ] ) diff --git a/EasyReflectometryApp/Backends/Py/logic/assemblies.py b/EasyReflectometryApp/Backends/Py/logic/assemblies.py index c1923a25..c999d60f 100644 --- a/EasyReflectometryApp/Backends/Py/logic/assemblies.py +++ b/EasyReflectometryApp/Backends/Py/logic/assemblies.py @@ -177,22 +177,5 @@ def set_conformal_roughness(self, new_value: bool) -> bool: def _from_assemblies_collection_to_list_of_dicts(assemblies_collection: Sample) -> list[dict[str, str]]: - assemblies_list = [] - for assembly in assemblies_collection: - assemblies_list.append( - { - 'label': assembly.name, - 'type': assembly.type, - 'repetitions': 1, - 'constrain_apm': 'False', - 'conformal_roughness': 'False', - } - ) - if isinstance(assembly, RepeatingMultilayer): - assemblies_list[-1]['repetitions'] = assembly.repetitions - - if isinstance(assembly, SurfactantLayer): - assemblies_list[-1]['constrain_apm'] = assembly.constrain_area_per_molecule - assemblies_list[-1]['conformal_roughness'] = assembly.conformal_roughness - - return assemblies_list + # Repetitions and the surfactant flags have their own current-index properties. + return [{'label': assembly.name, 'type': assembly.type} for assembly in assemblies_collection] diff --git a/EasyReflectometryApp/Backends/Py/logic/bayesian.py b/EasyReflectometryApp/Backends/Py/logic/bayesian.py index 6dc3aad2..9afc61e4 100644 --- a/EasyReflectometryApp/Backends/Py/logic/bayesian.py +++ b/EasyReflectometryApp/Backends/Py/logic/bayesian.py @@ -1,6 +1,6 @@ """State container for Bayesian DREAM hyper-parameters and posterior results.""" -_VALID_INITIALIZERS = ('eps', 'cov', 'lhs', 'random') +VALID_INITIALIZERS = ('eps', 'cov', 'lhs', 'random') class Bayesian: @@ -80,11 +80,11 @@ def initializer(self) -> str: @initializer.setter def initializer(self, value: str) -> None: - if value in _VALID_INITIALIZERS: + if value in VALID_INITIALIZERS: self._initializer = value else: raise ValueError( - f'Unknown initializer {value!r}. Valid options: {_VALID_INITIALIZERS}' + f'Unknown initializer {value!r}. Valid options: {VALID_INITIALIZERS}' ) # ------------------------------------------------------------------ diff --git a/EasyReflectometryApp/Backends/Py/logic/layers.py b/EasyReflectometryApp/Backends/Py/logic/layers.py index 5b048dbc..8fcf35b5 100644 --- a/EasyReflectometryApp/Backends/Py/logic/layers.py +++ b/EasyReflectometryApp/Backends/Py/logic/layers.py @@ -53,7 +53,7 @@ def name_at_current_index(self) -> str: return self._layers[self.index].name @property - def layers(self) -> list[dict[str, str]]: + def layers(self) -> list[dict[str, str | bool]]: return _from_layers_collection_to_list_of_dicts(self._layers) @property @@ -66,14 +66,17 @@ def remove_at_index(self, index: int) -> None: # model's field, acting on nothing; the project fixes those. self._project_lib._sync_parameter_states() - def add_new(self) -> None: - if 'Si' not in [material.name for material in self._project_lib._materials]: + def add_new(self) -> bool: + """Append a Si layer; return whether the Si material had to be added to the project.""" + material_added = 'Si' not in [material.name for material in self._project_lib._materials] + if material_added: self._project_lib._materials.add_material(Material(name='Si', sld=2.07, isld=0.0)) index_si = [material.name for material in self._project_lib._materials].index('Si') self._layers.add_layer() self._layers[-1].material = self._project_lib._materials[index_si] # Set layer name based on material name self._layers[-1].name = self._project_lib._materials[index_si].name + ' Layer' + return material_added def duplicate_selected(self) -> None: self._layers.duplicate_layer(self.index) @@ -381,7 +384,7 @@ def _set_parameter_value(parameter, new_value) -> bool: return True -def _from_layers_collection_to_list_of_dicts(layers_collection: LayerCollection) -> list[dict[str, str]]: +def _from_layers_collection_to_list_of_dicts(layers_collection: LayerCollection) -> list[dict[str, str | bool]]: """Convert layers collection to list of dicts. Editability of thickness and roughness mirrors each parameter's ``enabled`` @@ -389,7 +392,8 @@ def _from_layers_collection_to_list_of_dicts(layers_collection: LayerCollection) 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). + parameters in agreement (issue #329). Only area-per-molecule layers carry + ``formula``, ``apm``, ``solvent`` and ``solvation``. :param layers_collection: The collection of layers. """ @@ -401,15 +405,10 @@ def _from_layers_collection_to_list_of_dicts(layers_collection: LayerCollection) 'roughness': str(layer.roughness.value), 'thickness': str(layer.thickness.value), 'material': layer.material.name, - 'formula': 'formula', - 'apm': '0.1', - 'solvent': 'solvent', - 'solvation': '0.2', - 'apm_enabled': 'True', # `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))), + 'thickness_enabled': bool(getattr(layer.thickness, 'enabled', True)), + 'roughness_enabled': bool(getattr(layer.roughness, 'enabled', True)), } ) if isinstance(layer, LayerAreaPerMolecule): diff --git a/EasyReflectometryApp/Backends/Py/logic/minimizers.py b/EasyReflectometryApp/Backends/Py/logic/minimizers.py index 3f363082..db8f0769 100644 --- a/EasyReflectometryApp/Backends/Py/logic/minimizers.py +++ b/EasyReflectometryApp/Backends/Py/logic/minimizers.py @@ -28,8 +28,8 @@ def __init__(self, project_lib: ProjectLib): self._project_lib = project_lib # Aliases of a listed member (same engine and method) are not offered. aliases = [AvailableMinimizers.__members__.get(name) for name in ('LMFit', 'Bumps', 'DFO')] - # Index 0 is the Bayesian sentinel (None), which requires an explicit user choice. - self._list_available_minimizers = [None] + [m for m in AvailableMinimizers if not any(m is alias for alias in aliases)] + # The GUI list is the Bayesian choice at index 0, then these: minimizer i is at index i + 1. + self._minimizers = [m for m in AvailableMinimizers if not any(m is alias for alias in aliases)] @property def _settings(self) -> FitSettings: @@ -37,18 +37,18 @@ def _settings(self) -> FitSettings: return self._project_lib.fit_settings def minimizers_available(self) -> list[str]: - return [BAYESIAN_LABEL if m is None else m.name for m in self._list_available_minimizers] + return [BAYESIAN_LABEL] + [m.name for m in self._minimizers] def minimizer_current_index(self) -> int: if self._settings.mode == 'sample': return 0 selected = self._settings.minimizer - for index, entry in enumerate(self._list_available_minimizers): + for index, entry in enumerate(self._minimizers, start=1): if entry is selected: return index # An alias set directly on the project: the listed member with the same engine and method. - for index, entry in enumerate(self._list_available_minimizers): - if entry is not None and (entry.package, entry.method) == (selected.package, selected.method): + for index, entry in enumerate(self._minimizers, start=1): + if (entry.package, entry.method) == (selected.package, selected.method): return index return 1 @@ -62,7 +62,7 @@ def supports_inequalities(self) -> bool: """Whether the engine that will actually run the fit can enforce inequality constraints. Inequalities are BUMPS penalties: every ``Bumps*`` method and the DREAM - sampler (the Bayesian sentinel resolves to ``Bumps_simplex``) qualify; + sampler (the Bayesian choice runs on ``Bumps_simplex``) qualify; LMFit and DFO-LS do not. """ if self.is_bayesian_selected(): @@ -83,15 +83,14 @@ def requires_finite_bounds(self) -> bool: return not self.is_bayesian_selected() and requires_finite_bounds(self._settings.minimizer) def set_minimizer_current_index(self, new_value: int) -> bool: - if not 0 <= new_value < len(self._list_available_minimizers) or new_value == self.minimizer_current_index(): + if not 0 <= new_value <= len(self._minimizers) or new_value == self.minimizer_current_index(): return False - entry = self._list_available_minimizers[new_value] - if entry is None: + if new_value == 0: # Bayesian mode: the sampler needs a BUMPS engine. self._settings.minimizer = AvailableMinimizers.Bumps_simplex self._settings.mode = 'sample' else: - self._settings.minimizer = entry + self._settings.minimizer = self._minimizers[new_value - 1] self._settings.mode = 'minimize' return True diff --git a/EasyReflectometryApp/Backends/Py/logic/parameters.py b/EasyReflectometryApp/Backends/Py/logic/parameters.py index d3756f2a..964486e5 100644 --- a/EasyReflectometryApp/Backends/Py/logic/parameters.py +++ b/EasyReflectometryApp/Backends/Py/logic/parameters.py @@ -6,6 +6,8 @@ from typing import Tuple from easyreflectometry import Project as ProjectLib +from easyreflectometry.sample import Layer +from easyreflectometry.sample import LayerMagnetism from easyreflectometry.utils import count_fixed_parameters from easyreflectometry.utils import count_free_parameters from easyscience.base_classes import ModelBase @@ -65,9 +67,6 @@ def set_variability_filter_criteria(self, criteria: str) -> bool: self._current_index = 0 return True - def is_experiment_parameter(self, parameter: dict[str, Any]) -> bool: - return _is_experiment_parameter(parameter) - def _parameter_matches_filters(self, parameter: dict[str, Any]) -> bool: if not parameter.get('enabled', True): return False @@ -87,9 +86,9 @@ def _parameter_matches_filters(self, parameter: dict[str, Any]) -> bool: ).lower() if normalized == 'model': - return not _is_experiment_parameter(parameter) + return not parameter['experiment'] if normalized == 'experiment': - return _is_experiment_parameter(parameter) + return parameter['experiment'] if normalized in {'magnetic', 'magnetism'}: # The magnetic parameters are named after the physics, so the word # rarely appears in their text; match them explicitly. @@ -259,12 +258,6 @@ def _from_parameters_to_list_of_dicts(parameters: List[Parameter], models) -> li alias_registry: set[str] = set() processed_unique_names: set[str] = set() # Track processed parameters to avoid duplicates - # Layer parameter names that need model prefix - LAYER_PARAMS = {'thickness', 'roughness'} - # Magnetism sits one level below the layer (Layer -> LayerMagnetism -> param), - # but belongs to the layer just as much, so it is named the same way. - MAGNETISM_PARAMS = {'rho_m', 'theta_m'} - def _make_alias(name: str) -> str: base = re.sub(r'[^0-9A-Za-z]+', '_', name).strip('_').lower() if not base: @@ -291,9 +284,9 @@ def _get_parameter_display_data(param: Parameter, path: list) -> Tuple[str, str] # For layer parameters the path is: # Model -> Sample -> Assembly -> LayerCollection -> Layer -> param # Use the assembly name (path[-4]) instead of the layer name (path[-2]) - if _is_layer_parameter(param) and len(path) >= 4: + if _is_layer_parameter(param, path) and len(path) >= 4: parent_name = path[-4].name - elif _is_magnetism_parameter(param) and len(path) >= 5: + elif _is_magnetism_parameter(path) and len(path) >= 5: # ... -> Assembly -> LayerCollection -> Layer -> LayerMagnetism -> param: # one level deeper, so the assembly is path[-5]. Without this the # group would read 'EasyLayerMagnetism', which names nothing. @@ -320,17 +313,22 @@ def _get_dependency_expression(param: Parameter, paths: dict) -> str: # Simple numerical dependency return f'= {param.value}' - def _is_layer_parameter(param: Parameter) -> bool: - """Check if parameter is a layer parameter (thickness or roughness).""" - return param.name.lower() in LAYER_PARAMS + def _is_layer_parameter(param: Parameter, path: list) -> bool: + """Check if parameter is a layer's own thickness or roughness.""" + owner = path[-2] + return isinstance(owner, Layer) and (param is owner.thickness or param is owner.roughness) - def _is_magnetism_parameter(param: Parameter) -> bool: - """Check if parameter is a layer magnetism parameter (rho_m or theta_m).""" - return param.name.lower() in MAGNETISM_PARAMS + def _is_magnetism_parameter(path: list) -> bool: + """Check if parameter belongs to a layer's magnetism (rho_m or theta_m). + + Magnetism sits one level below the layer (Layer -> LayerMagnetism -> param), + but belongs to the layer just as much, so it is named the same way. + """ + return isinstance(path[-2], LayerMagnetism) - def _is_per_layer_parameter(param: Parameter) -> bool: + def _is_per_layer_parameter(param: Parameter, path: list) -> bool: """Per-layer parameters exist once per model and carry the model prefix.""" - return _is_layer_parameter(param) or _is_magnetism_parameter(param) + return _is_layer_parameter(param, path) or _is_magnetism_parameter(path) parameter_list = [] @@ -362,7 +360,7 @@ def _is_per_layer_parameter(param: Parameter) -> bool: continue # Each parameter once, whichever models share it - is_layer_param = _is_per_layer_parameter(parameter) + is_layer_param = _is_per_layer_parameter(parameter, path) if parameter.unique_name in processed_unique_names: continue processed_unique_names.add(parameter.unique_name) @@ -428,6 +426,7 @@ def _is_per_layer_parameter(param: Parameter) -> bool: 'detachable': len(shared_by) > 1 and is_layer_param and parameter.independent, 'followedBy': followed_by.get(parameter.unique_name, 0), 'object': parameter, # Direct reference to the Parameter object + 'experiment': _is_experiment_parameter(parameter, models[model_idx]), 'exclusive_with': _exclusive_partner(parameter, path), } ) @@ -550,16 +549,6 @@ def _visit(obj, chain: list) -> None: return paths -def _is_experiment_parameter(parameter: dict[str, Any]) -> bool: - searchable_text = ' '.join(str(parameter.get(key, '')) for key in ('name', 'display_name', 'group', 'unique_name')).lower() - experiment_markers = ( - 'experiment', - 'dataset', - 'instrument', - 'resolution', - 'asymmetry', - 'background', - 'scale', - 'probe', - ) - return any(marker in searchable_text for marker in experiment_markers) +def _is_experiment_parameter(parameter: Parameter, model) -> bool: + """True for the model's instrument-side parameters (scale, background), as opposed to the sample's.""" + return parameter is model.scale or parameter is model.background diff --git a/EasyReflectometryApp/Backends/Py/logic/project.py b/EasyReflectometryApp/Backends/Py/logic/project.py index 517539b1..da97c843 100644 --- a/EasyReflectometryApp/Backends/Py/logic/project.py +++ b/EasyReflectometryApp/Backends/Py/logic/project.py @@ -64,6 +64,8 @@ def description(self, new_value: str) -> None: @property def creation_date(self) -> str: + # Despite its key, the library stamps 'modified' only in `Project.create()` + # (saving leaves it alone), so it holds the creation time. return self._project_lib._info['modified'] @property diff --git a/EasyReflectometryApp/Backends/Py/project.py b/EasyReflectometryApp/Backends/Py/project.py index 5ac18bc3..81761b64 100644 --- a/EasyReflectometryApp/Backends/Py/project.py +++ b/EasyReflectometryApp/Backends/Py/project.py @@ -75,7 +75,7 @@ def lastSaved(self) -> str: """ISO-8601 time of the last successful save, with UTC offset, or '' if never saved. This is the time of the last save made from this session, which is deliberately not - the same as `creationDate` (the project's stored modification stamp). The value is + the same as `creationDate` (the stamp stored when the project was created). The value is left unformatted so that QML can render it with a locale-aware `Qt.formatTime`. """ return self._last_saved diff --git a/EasyReflectometryApp/Backends/Py/sample.py b/EasyReflectometryApp/Backends/Py/sample.py index f5823084..e35946c9 100644 --- a/EasyReflectometryApp/Backends/Py/sample.py +++ b/EasyReflectometryApp/Backends/Py/sample.py @@ -632,7 +632,7 @@ def layersConnectChanges(self) -> None: self._clearCacheAndEmitLayersChanged() @Property('QVariantList', notify=layersChanged) - def layers(self) -> list[dict[str, str]]: + def layers(self) -> list[dict[str, str | bool]]: if self._cached_layers is None: self._cached_layers = self._layers_logic.layers return self._cached_layers @@ -783,7 +783,8 @@ def removeLayer(self, index: int) -> None: @Slot() def addNewLayer(self) -> None: - self._layers_logic.add_new() + if self._layers_logic.add_new(): + self.materialsTableChanged.emit() self._project_logic._update_enablement_of_fixed_layers_for_model(self._models_logic.index) self._clearCacheAndEmitLayersChanged() self.externalRefreshPlot.emit() @@ -1108,11 +1109,11 @@ def _sanitize_relation(operator: str) -> str: def _format_numeric(value: float) -> str: return f'{value:.6g}' - def _get_independent_parameter_entries(self) -> list[dict]: - """Return the filtered list of independent+enabled parameter entries. + def _constrainable_parameter_entries(self) -> list[dict]: + """The parameters a new constraint can make dependent: independent and enabled. - This must match the same filtering as dependentParameterNames so that - the QML dropdown index maps to the correct parameter object. + ``constrainableParameterNames`` lists their names, so an index into the + QML dropdown is an index into this list. """ entries = [] for parameter in self._parameters_logic.parameters: @@ -1129,7 +1130,7 @@ def _prepare_constraint_instruction( relation_operator: str, expression: str, ) -> dict[str, Any]: - independent_entries = self._get_independent_parameter_entries() + independent_entries = self._constrainable_parameter_entries() if dependent_index < 0 or dependent_index >= len(independent_entries): raise ValueError('Select a dependent parameter before defining a constraint.') @@ -1463,15 +1464,8 @@ def enabledParameterNames(self) -> list[str]: return enabled_param_names @Property('QVariantList', notify=layersChanged) - def dependentParameterNames(self) -> list[str]: - dep_param_names = [] - for parameter in self._parameters_logic.parameters: - if not parameter['independent']: - continue - if hasattr(parameter['object'], 'enabled') and not parameter['object'].enabled: - continue - dep_param_names.append(parameter['name']) - return dep_param_names + def constrainableParameterNames(self) -> list[str]: + return [parameter['name'] for parameter in self._constrainable_parameter_entries()] @Property('QVariantList', notify=layersChanged) def constraintParametersMetadata(self) -> list[dict[str, Any]]: @@ -1963,7 +1957,7 @@ def addConstraint(self, dependent_index: int, relation: str, expression: str): 'warning': instruction.get('warning', ''), } - dependent = self._get_independent_parameter_entries()[dependent_index]['object'] + dependent = self._constrainable_parameter_entries()[dependent_index]['object'] previous_state = self._capture_parameter_state(dependent) self._ensure_parameter_independent(dependent) diff --git a/EasyReflectometryApp/Gui/Globals/BackendWrapper.qml b/EasyReflectometryApp/Gui/Globals/BackendWrapper.qml index e6b4e9e2..5ba79492 100644 --- a/EasyReflectometryApp/Gui/Globals/BackendWrapper.qml +++ b/EasyReflectometryApp/Gui/Globals/BackendWrapper.qml @@ -312,7 +312,7 @@ QtObject { // Constraints readonly property var sampleEnabledParameterNames: activeBackend.sample.enabledParameterNames readonly property var sampleParameterNames: activeBackend.sample.parameterNames - readonly property var sampleDepParameterNames: activeBackend.sample.dependentParameterNames + readonly property var sampleConstrainableParameterNames: activeBackend.sample.constrainableParameterNames readonly property var sampleRelationOperators: activeBackend.sample.relationOperators readonly property var sampleArithmeticOperators: activeBackend.sample.arithmeticOperators readonly property var sampleConstraintsList: activeBackend.sample.constraintsList diff --git a/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Advanced/Groups/Constraints.qml b/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Advanced/Groups/Constraints.qml index 3a1c5273..b28dbc73 100644 --- a/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Advanced/Groups/Constraints.qml +++ b/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Advanced/Groups/Constraints.qml @@ -198,7 +198,7 @@ EaElements.GroupBox { width: Math.max(0, parameterRow.width - relationalOperator.width - parameterRow.spacing) currentIndex: -1 displayText: currentIndex === -1 ? qsTr("Select dependent parameter") : currentText - model: Globals.BackendWrapper.sampleDepParameterNames + model: Globals.BackendWrapper.sampleConstrainableParameterNames onCurrentIndexChanged: constraintsGroup.scheduleValidation() } diff --git a/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Assemblies/MultiLayer.qml b/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Assemblies/MultiLayer.qml index ccb9a056..34a002e2 100644 --- a/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Assemblies/MultiLayer.qml +++ b/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Assemblies/MultiLayer.qml @@ -77,7 +77,7 @@ EaElements.GroupColumn { EaComponents.TableViewTextInput { horizontalAlignment: Text.AlignHCenter - enabled: Globals.BackendWrapper.sampleLayers[index].thickness_enabled === "True" + enabled: Globals.BackendWrapper.sampleLayers[index].thickness_enabled text: (isNaN(Globals.BackendWrapper.sampleLayers[index].thickness)) ? '--' : Number(Globals.BackendWrapper.sampleLayers[index].thickness).toFixed(2) onActiveFocusChanged: if (activeFocus && Globals.BackendWrapper.sampleCurrentLayerIndex !== index) Globals.BackendWrapper.sampleSetCurrentLayerIndex(index) onEditingFinished: Globals.BackendWrapper.sampleSetLayerThicknessAtIndex(index, text) @@ -85,7 +85,7 @@ EaElements.GroupColumn { EaComponents.TableViewTextInput { horizontalAlignment: Text.AlignHCenter - enabled: Globals.BackendWrapper.sampleLayers[index].roughness_enabled === "True" + enabled: Globals.BackendWrapper.sampleLayers[index].roughness_enabled text: (isNaN(Globals.BackendWrapper.sampleLayers[index].roughness)) ? '--' : Number(Globals.BackendWrapper.sampleLayers[index].roughness).toFixed(2) onActiveFocusChanged: if (activeFocus && Globals.BackendWrapper.sampleCurrentLayerIndex !== index) Globals.BackendWrapper.sampleSetCurrentLayerIndex(index) onEditingFinished: Globals.BackendWrapper.sampleSetLayerRoughnessAtIndex(index, text) diff --git a/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Assemblies/SurfactantLayer.qml b/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Assemblies/SurfactantLayer.qml index 62911131..a87d04b7 100644 --- a/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Assemblies/SurfactantLayer.qml +++ b/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Assemblies/SurfactantLayer.qml @@ -66,7 +66,7 @@ EaElements.GroupColumn { EaComponents.TableViewTextInput { horizontalAlignment: Text.AlignHCenter - enabled: Globals.BackendWrapper.sampleLayers[index].thickness_enabled === "True" + enabled: Globals.BackendWrapper.sampleLayers[index].thickness_enabled text: (isNaN(Globals.BackendWrapper.sampleLayers[index].thickness)) ? '--' : Number(Globals.BackendWrapper.sampleLayers[index].thickness).toFixed(2) onActiveFocusChanged: if (activeFocus && Globals.BackendWrapper.sampleCurrentLayerIndex !== index) Globals.BackendWrapper.sampleSetCurrentLayerIndex(index) onEditingFinished: Globals.BackendWrapper.sampleSetLayerThicknessAtIndex(index, text) @@ -74,7 +74,7 @@ EaElements.GroupColumn { EaComponents.TableViewTextInput { horizontalAlignment: Text.AlignHCenter - enabled: Globals.BackendWrapper.sampleLayers[index].roughness_enabled === "True" + enabled: Globals.BackendWrapper.sampleLayers[index].roughness_enabled text: (isNaN(Globals.BackendWrapper.sampleLayers[index].roughness)) ? '--' : Number(Globals.BackendWrapper.sampleLayers[index].roughness).toFixed(2) onActiveFocusChanged: if (activeFocus && Globals.BackendWrapper.sampleCurrentLayerIndex !== index) Globals.BackendWrapper.sampleSetCurrentLayerIndex(index) onEditingFinished: Globals.BackendWrapper.sampleSetLayerRoughnessAtIndex(index, text) @@ -89,7 +89,6 @@ EaElements.GroupColumn { EaComponents.TableViewTextInput { horizontalAlignment: Text.AlignHCenter - enabled: Globals.BackendWrapper.sampleLayers[index].apm_enabled === "True" text: (isNaN(Globals.BackendWrapper.sampleLayers[index].apm)) ? '--' : Number(Globals.BackendWrapper.sampleLayers[index].apm).toFixed(2) onActiveFocusChanged: if (activeFocus && Globals.BackendWrapper.sampleCurrentLayerIndex !== index) Globals.BackendWrapper.sampleSetCurrentLayerIndex(index) onEditingFinished: Globals.BackendWrapper.sampleSetLayerAPMAtIndex(index, text) diff --git a/tests/test_analysis_bayesian.py b/tests/test_analysis_bayesian.py index 1317d7ca..c0bd5905 100644 --- a/tests/test_analysis_bayesian.py +++ b/tests/test_analysis_bayesian.py @@ -466,13 +466,10 @@ class TestBayesianCornerPlotUrl: def test_empty_when_no_posterior(self, analysis): assert analysis.bayesianCornerPlotUrl == '' - def test_lazy_renders_once(self, analysis_with_posterior): - # Pre-set a cached URL so the second access doesn't re-trigger rendering - analysis_with_posterior._bayesian_logic.corner_plot_url = 'file:///cached.png' + def test_reading_never_renders(self, analysis_with_posterior): with patch.object(analysis_with_posterior, '_render_corner_plot') as mock: - url1 = analysis_with_posterior.bayesianCornerPlotUrl - url2 = analysis_with_posterior.bayesianCornerPlotUrl - assert mock.call_count == 0 # already cached, no call needed + assert analysis_with_posterior.bayesianCornerPlotUrl == '' + mock.assert_not_called() def test_returns_cached_url(self, analysis_with_posterior): analysis_with_posterior._bayesian_logic.corner_plot_url = 'file:///cached.png' @@ -488,13 +485,10 @@ class TestBayesianTracePlotUrl: def test_empty_when_no_posterior(self, analysis): assert analysis.bayesianTracePlotUrl == '' - def test_lazy_renders_once(self, analysis_with_posterior): - # Pre-set a cached URL so the second access doesn't re-trigger rendering - analysis_with_posterior._bayesian_logic.trace_plot_url = 'file:///cached_trace.png' + def test_reading_never_renders(self, analysis_with_posterior): with patch.object(analysis_with_posterior, '_render_trace_plot') as mock: - url1 = analysis_with_posterior.bayesianTracePlotUrl - url2 = analysis_with_posterior.bayesianTracePlotUrl - assert mock.call_count == 0 # already cached, no call needed + assert analysis_with_posterior.bayesianTracePlotUrl == '' + mock.assert_not_called() def test_returns_cached_url(self, analysis_with_posterior): analysis_with_posterior._bayesian_logic.trace_plot_url = 'file:///cached_trace.png' @@ -510,12 +504,10 @@ class TestBayesianDistributionPlotUrl: def test_empty_when_no_posterior(self, analysis): assert analysis.bayesianDistributionPlotUrl == '' - def test_lazy_renders_once(self, analysis_with_posterior): - analysis_with_posterior._bayesian_logic.distribution_plot_url = 'file:///cached_dist.html' + def test_reading_never_renders(self, analysis_with_posterior): with patch.object(analysis_with_posterior, '_render_distribution_plot') as mock: - url1 = analysis_with_posterior.bayesianDistributionPlotUrl - url2 = analysis_with_posterior.bayesianDistributionPlotUrl - assert mock.call_count == 0 # already cached, no call needed + assert analysis_with_posterior.bayesianDistributionPlotUrl == '' + mock.assert_not_called() def test_returns_cached_url(self, analysis_with_posterior): analysis_with_posterior._bayesian_logic.distribution_plot_url = 'file:///cached_dist.html' @@ -958,12 +950,15 @@ def test_calls_posterior_predictive_and_diagnostics(self, analysis): with patch.object(analysis, '_compute_and_publish_posterior_predictive') as mock_pp: with patch.object(analysis, '_compute_diagnostics') as mock_diag: with patch.object(analysis, '_render_corner_plot') as mock_corner: - with patch.object(analysis, '_render_trace_plot') as mock_trace: + with patch.object(analysis, '_render_trace_plot') as mock_trace, patch.object( + analysis, '_render_distribution_plot' + ) as mock_distribution: analysis._on_sample_finished([SAMPLE_POSTERIOR_2D]) mock_pp.assert_called_once() mock_diag.assert_called_once() mock_corner.assert_called_once() mock_trace.assert_called_once() + mock_distribution.assert_called_once() def test_emits_fitting_signals(self, analysis): emissions = {'fitting': 0, 'external': 0} @@ -992,11 +987,14 @@ def test_failing_step_does_not_skip_the_others(self, analysis): with patch.object(analysis, '_compute_and_publish_posterior_predictive', side_effect=RuntimeError('boom')): with patch.object(analysis, '_compute_diagnostics') as mock_diag: with patch.object(analysis, '_render_corner_plot') as mock_corner: - with patch.object(analysis, '_render_trace_plot') as mock_trace: + with patch.object(analysis, '_render_trace_plot') as mock_trace, patch.object( + analysis, '_render_distribution_plot' + ) as mock_distribution: analysis._on_sample_finished([SAMPLE_POSTERIOR_2D]) mock_diag.assert_called_once() mock_corner.assert_called_once() mock_trace.assert_called_once() + mock_distribution.assert_called_once() assert analysis._bayesian_logic.posterior is SAMPLE_POSTERIOR_2D diff --git a/tests/test_logic_assemblies.py b/tests/test_logic_assemblies.py index 83730767..f2fb001c 100644 --- a/tests/test_logic_assemblies.py +++ b/tests/test_logic_assemblies.py @@ -12,21 +12,17 @@ from tests.factories import make_sample -def test_from_assemblies_collection_to_list_of_dicts_includes_special_fields(monkeypatch): - monkeypatch.setattr(assemblies_module, 'RepeatingMultilayer', FakeRepeatingMultilayer) - monkeypatch.setattr(assemblies_module, 'SurfactantLayer', FakeSurfactantLayer) - +def test_from_assemblies_collection_to_list_of_dicts_has_the_same_keys_for_every_type(): repeating = FakeRepeatingMultilayer(repetitions=3, name='Repeat') surfactant = FakeSurfactantLayer(name='Surf') - surfactant.constrain_area_per_molecule = 'True' - surfactant.conformal_roughness = 'True' result = assemblies_module._from_assemblies_collection_to_list_of_dicts([FakeMultilayer(name='Plain'), repeating, surfactant]) - assert result[0]['type'] == 'Multi-layer' - assert result[1]['repetitions'].value == 3 - assert result[2]['constrain_apm'] == 'True' - assert result[2]['conformal_roughness'] == 'True' + assert result == [ + {'label': 'Plain', 'type': 'Multi-layer'}, + {'label': 'Repeat', 'type': repeating.type}, + {'label': 'Surf', 'type': surfactant.type}, + ] def test_assemblies_add_new_and_type_transitions(monkeypatch): diff --git a/tests/test_logic_layers.py b/tests/test_logic_layers.py index 4f88a42b..fa35256e 100644 --- a/tests/test_logic_layers.py +++ b/tests/test_logic_layers.py @@ -26,8 +26,8 @@ def test_from_layers_collection_to_list_of_dicts_mirrors_parameter_enablement(mo ) 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]['thickness_enabled'] is True + assert regular[0]['roughness_enabled'] is True assert regular[0]['formula'] == 'C12H25' assert regular[0]['apm'] == '44.0' assert regular[0]['solvent'] == 'Water' @@ -36,13 +36,21 @@ def test_from_layers_collection_to_list_of_dicts_mirrors_parameter_enablement(mo 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' + assert superphase[0]['thickness_enabled'] is False + assert superphase[0]['roughness_enabled'] is 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' + assert subphase[0]['thickness_enabled'] is False + assert subphase[0]['roughness_enabled'] is True + + +def test_from_layers_collection_to_list_of_dicts_gives_plain_layers_no_apm_placeholders(): + layer = make_layer(name='Film', material=make_material('Si')) + + row = layers_module._from_layers_collection_to_list_of_dicts(make_layer_collection(layer))[0] + + assert set(row) == {'label', 'roughness', 'thickness', 'material', 'thickness_enabled', 'roughness_enabled'} def test_layers_add_new_creates_si_material_when_missing(monkeypatch): @@ -58,12 +66,15 @@ def test_layers_add_new_creates_si_material_when_missing(monkeypatch): project.current_assembly_index = 1 logic = layers_module.Layers(project) - logic.add_new() + assert logic.add_new() is True assert [material.name for material in project._materials] == ['Air', 'D2O', 'Si'] assert logic._layers[-1].material.name == 'Si' assert logic._layers[-1].name == 'Si Layer' + assert logic.add_new() is False # Si exists now and is reused + assert [material.name for material in project._materials] == ['Air', 'D2O', 'Si'] + def test_layers_move_duplicate_and_setters_update_current_layer(monkeypatch): monkeypatch.setattr(layers_module, 'Material', make_material) diff --git a/tests/test_logic_parameters.py b/tests/test_logic_parameters.py index 2d3bdfd3..28163e44 100644 --- a/tests/test_logic_parameters.py +++ b/tests/test_logic_parameters.py @@ -23,11 +23,22 @@ def __init__(self, name, unique_name, **children): setattr(self, key, value) +class FakeLayerNode(FakeNode): + """Stand-in for a library ``Layer``: its thickness and roughness are per-layer parameters.""" + + +class FakeMagnetismNode(FakeNode): + """Stand-in for a library ``LayerMagnetism``.""" + + def _patch_tree_types(monkeypatch): # The traversal uses module-level `Parameter`/`ModelBase` for isinstance - # checks; point them at the test fakes so the fake tree is walkable. + # checks, and `Layer`/`LayerMagnetism` to classify parameters; point them + # at the test fakes so the fake tree is walkable. monkeypatch.setattr(parameters_module, 'Parameter', FakeParameter) monkeypatch.setattr(parameters_module, 'ModelBase', FakeNode) + monkeypatch.setattr(parameters_module, 'Layer', FakeLayerNode) + monkeypatch.setattr(parameters_module, 'LayerMagnetism', FakeMagnetismNode) def test_from_parameters_to_list_of_dicts_marks_decoupled_density_knobs_inactive(monkeypatch): @@ -42,7 +53,7 @@ def test_from_parameters_to_list_of_dicts_marks_decoupled_density_knobs_inactive def build_model(sld_coupled): model = make_model(name='M1 internal', unique_name='m1', user_data={'original_name': 'M1'}) - layer = FakeNode('Layer', 'm1_layer', thickness=thickness) + layer = FakeLayerNode('Layer', 'm1_layer', thickness=thickness, roughness=None) assembly = FakeNode('LayerA', 'm1_asm', layers=[layer]) model.sample = [assembly] # Density-material knobs live under the material node, which carries @@ -96,7 +107,7 @@ def build_model(unique_name, original_name, with_background): name=f'{original_name} internal', unique_name=unique_name, user_data={'original_name': original_name} ) # Model -> sample -> assembly -> layers -> layer -> thickness (a layer parameter) - layer = FakeNode('Layer', f'{unique_name}_layer', thickness=thickness) + layer = FakeLayerNode('Layer', f'{unique_name}_layer', thickness=thickness, roughness=None) assembly = FakeNode('LayerA', f'{unique_name}_asm', layers=[layer]) model.sample = [assembly] # Non-layer params (scale, background) live under a shared 'Instrument' parent @@ -139,7 +150,7 @@ def test_layer_alias_attributes_do_not_shortcut_the_canonical_path(monkeypatch): roughness = make_parameter(name='roughness', unique_name='roughness', value=3.0, free=True, enabled=True) model = make_model(name='M1 internal', unique_name='m1', user_data={'original_name': 'M1'}) - layer = FakeNode('Layer', 'layer', thickness=thickness, roughness=roughness) + layer = FakeLayerNode('Layer', 'layer', thickness=thickness, roughness=roughness) assembly = FakeNode( 'Superphase', 'asm', back_layer=layer, # alias, sorts before 'layers' @@ -158,6 +169,37 @@ def test_layer_alias_attributes_do_not_shortcut_the_canonical_path(monkeypatch): assert all(entry['group'] == 'Superphase' for entry in result) +def test_from_parameters_to_list_of_dicts_classifies_by_owner_not_by_name(monkeypatch): + _patch_tree_types(monkeypatch) + + scale = make_parameter(name='scale', unique_name='scale') + background = make_parameter(name='background', unique_name='background') + thickness = make_parameter(name='thickness', unique_name='thickness') + rho_m = make_parameter(name='rho_m', unique_name='rho_m') + # Named like experiment and layer parameters, but owned by a material. + impostor = make_parameter(name='thickness', unique_name='impostor') + scale_named = make_parameter(name='scale', unique_name='scale_named') + + model = make_model(name='M1 internal', unique_name='m1', user_data={'original_name': 'M1'}) + model.scale = scale + model.background = background + magnetism = FakeMagnetismNode('EasyLayerMagnetism', 'mag', rho_m=rho_m) + layer = FakeLayerNode('Layer', 'layer', thickness=thickness, roughness=None, magnetism=magnetism) + model.sample = [FakeNode('Film', 'asm', layers=[layer])] + model.material = FakeNode('Background Material', 'mat', thickness=impostor, scale=scale_named) + + result = parameters_module._from_parameters_to_list_of_dicts( + [scale, background, thickness, rho_m, impostor, scale_named], make_model_collection(model) + ) + rows = {entry['unique_name']: entry for entry in result} + + assert {name for name, row in rows.items() if row['experiment']} == {'scale', 'background'} + assert rows['thickness']['display_name'] == 'M1 Film thickness' + assert rows['rho_m']['display_name'] == 'M1 Film rho_m' + assert rows['impostor']['display_name'] == 'Background Material thickness' + assert rows['scale_named']['display_name'] == 'Background Material scale' + + def test_parameters_filtering_metadata_and_current_parameter_updates(monkeypatch): monkeypatch.setattr(parameters_module, 'count_free_parameters', lambda project: 2) monkeypatch.setattr(parameters_module, 'count_fixed_parameters', lambda project: 1) @@ -314,11 +356,11 @@ def test_parameters_special_filters_and_invalid_variability_normalization(monkey project = make_project() logic = parameters_module.Parameters(project) mocked_parameters = [ - {'name': 'Cell length a', 'display_name': 'Cell length a', 'group': 'cell', 'unique_name': 'cell.a', 'fit': True, 'enabled': True, 'independent': True, 'alias': 'cell_a', 'object': object()}, - {'name': 'Atom site x', 'display_name': 'Atom site x', 'group': 'atom_site', 'unique_name': 'atom_site.x', 'fit': False, 'enabled': True, 'independent': True, 'alias': 'atom_site_x', 'object': object()}, - {'name': 'Biso', 'display_name': 'adp b_iso', 'group': 'thermal', 'unique_name': 'b_iso', 'fit': False, 'enabled': True, 'independent': True, 'alias': 'b_iso', 'object': object()}, - {'name': 'Instrument scale', 'display_name': 'Instrument scale', 'group': 'instrument', 'unique_name': 'scale', 'fit': True, 'enabled': True, 'independent': True, 'alias': 'scale', 'object': object()}, - {'name': 'Layer thickness', 'display_name': 'Layer thickness', 'group': 'layer', 'unique_name': 'layer.thickness', 'fit': True, 'enabled': True, 'independent': True, 'alias': 'thickness', 'object': object()}, + {'name': 'Cell length a', 'display_name': 'Cell length a', 'group': 'cell', 'unique_name': 'cell.a', 'fit': True, 'enabled': True, 'independent': True, 'alias': 'cell_a', 'object': object(), 'experiment': False}, + {'name': 'Atom site x', 'display_name': 'Atom site x', 'group': 'atom_site', 'unique_name': 'atom_site.x', 'fit': False, 'enabled': True, 'independent': True, 'alias': 'atom_site_x', 'object': object(), 'experiment': False}, + {'name': 'Biso', 'display_name': 'adp b_iso', 'group': 'thermal', 'unique_name': 'b_iso', 'fit': False, 'enabled': True, 'independent': True, 'alias': 'b_iso', 'object': object(), 'experiment': False}, + {'name': 'Instrument scale', 'display_name': 'Instrument scale', 'group': 'instrument', 'unique_name': 'scale', 'fit': True, 'enabled': True, 'independent': True, 'alias': 'scale', 'object': object(), 'experiment': True}, + {'name': 'Layer thickness', 'display_name': 'Layer thickness', 'group': 'layer', 'unique_name': 'layer.thickness', 'fit': True, 'enabled': True, 'independent': True, 'alias': 'thickness', 'object': object(), 'experiment': False}, ] monkeypatch.setattr(logic, 'all_parameters', lambda: mocked_parameters) diff --git a/tests/test_py_sample_constraints.py b/tests/test_py_sample_constraints.py index 4980873f..3fbfe522 100644 --- a/tests/test_py_sample_constraints.py +++ b/tests/test_py_sample_constraints.py @@ -49,7 +49,7 @@ def project_and_backend(qcore_application): def _dependent_index(backend, text): - names = backend.dependentParameterNames + names = backend.constrainableParameterNames return next(i for i, name in enumerate(names) if all(part in name for part in text.split())) @@ -207,7 +207,7 @@ def test_bumps_and_bayesian_are_accepted(self, project_and_backend): self._select(minimizers, 'Bumps_simplex') assert minimizers.supports_inequalities() and fitting.inequality_constraints_error(minimizers) is None assert fitting.inequality_constraints_warning(minimizers) == '' - minimizers.set_minimizer_current_index(0) # Bayesian sentinel + minimizers.set_minimizer_current_index(0) # the Bayesian choice assert minimizers.is_bayesian_selected() and minimizers.supports_inequalities() assert fitting.inequality_constraints_error(minimizers) is None assert callable(fitting.snapshot_constraints_factory()) @@ -442,7 +442,7 @@ def test_static_constraint_reloads_pinned(self, project_and_backend): assert roughness.free is False assert roughness.independent is False # Pinned, so it is not offered as the dependent of a new constraint. - assert not any('Film A roughness' in name for name in reloaded_backend.dependentParameterNames) + assert not any('Film A roughness' in name for name in reloaded_backend.constrainableParameterNames) def test_bounds_reload_with_their_values(self, project_and_backend): project, backend = project_and_backend @@ -710,7 +710,7 @@ def _alias_of(backend, parameter): @staticmethod def _dependent_index_of(backend, parameter): - return next(i for i, entry in enumerate(backend._get_independent_parameter_entries()) if entry['object'] is parameter) + return next(i for i, entry in enumerate(backend._constrainable_parameter_entries()) if entry['object'] is parameter) @staticmethod def _constrain_film_a_sld_to_substrate(project, backend): diff --git a/tests/test_py_sample_fixed_layers.py b/tests/test_py_sample_fixed_layers.py index c8b0ab8d..44e371f2 100644 --- a/tests/test_py_sample_fixed_layers.py +++ b/tests/test_py_sample_fixed_layers.py @@ -49,8 +49,8 @@ def _assert_only_fixed_layers_disabled(project): 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] + assert [row['thickness_enabled'] for row in rows] == [bool(_enabled(layer.thickness)) for layer in layers] + assert [row['roughness_enabled'] for row in rows] == [bool(_enabled(layer.roughness)) for layer in layers] def test_table_treats_a_parameter_without_enablement_as_editable(project_and_backend): @@ -60,7 +60,7 @@ def test_table_treats_a_parameter_without_enablement_as_editable(project_and_bac backend.setCurrentAssemblyIndex(1) backend.layersConnectChanges() - assert backend.layers[0]['thickness_enabled'] == 'True' + assert backend.layers[0]['thickness_enabled'] is True class TestSuperphaseLayerEdits: @@ -153,11 +153,26 @@ def test_move_up_hands_the_subphase_back(self, project_and_backend): _assert_table_matches_parameters(project, backend) +def test_added_layer_announces_the_si_material_only_when_it_creates_it(project_and_backend): + project, backend = project_and_backend + backend.setCurrentAssemblyIndex(1) + emitted = [] + backend.materialsTableChanged.connect(lambda: emitted.append(True)) + + backend.addNewLayer() # the default project already has Si + assert emitted == [] + + next(material for material in project._materials if material.name == 'Si').name = 'Silicon' + backend.addNewLayer() + assert emitted == [True] + assert 'Si' in [material.name for material in project._materials] + + 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 all(row['thickness_enabled'] is True for row in backend.layers) + assert all(row['roughness_enabled'] is True for row in backend.layers) _assert_only_fixed_layers_disabled(project) From b015fb36590aa1c452113a8299eb4278528d6ecc Mon Sep 17 00:00:00 2001 From: rozyczko Date: Thu, 29 Oct 2026 18:12:51 +0100 Subject: [PATCH 2/2] more PR review fixes --- CHANGELOG.md | 12 ++++++ EasyReflectometryApp/Backends/Mock/Sample.qml | 9 +++-- .../Backends/Py/logic/assemblies.py | 10 +++-- .../Backends/Py/logic/layers.py | 40 +++++++++---------- .../Backends/Py/logic/parameters.py | 8 +++- EasyReflectometryApp/Backends/Py/sample.py | 7 ++-- .../Sample/Sidebar/Basic/Groups/Magnetism.qml | 16 ++++---- tests/factories.py | 3 ++ tests/test_logic_assemblies.py | 16 +++++++- tests/test_logic_layers.py | 17 +++----- tests/test_logic_minimizers.py | 24 +++++++++++ tests/test_logic_parameters.py | 38 ++++++++++++++---- tests/test_magnetic_material_and_field.py | 4 +- tests/test_magnetic_orso_load.py | 2 +- tests/test_polarized_analysis.py | 14 +++---- tests/test_py_sample_fixed_layers.py | 15 +++++++ tests/test_qml_magnetization_slider.py | 6 +-- 17 files changed, 171 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66c95b45..a2a77d81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,18 @@ assembly: thickness and roughness) and the subphase (last layer of the last assembly: thickness) are locked, the table shows exactly what the fit uses, and the lock is set again after every layer change. +- Adding a layer or an assembly to a project that has no `Si` material still + creates one for the new layer, and the Materials table now shows it at once + instead of after the next unrelated refresh. +- Bayesian results: the marginal-distribution plot is rendered together with the + corner and trace plots when sampling finishes, so its tab no longer renders on + first open, and a plot that failed to render is not retried on every redraw. +- Analysis parameter filters: **model** and **experiment** now go by what owns a + parameter. Only a model's scale and background are experiment parameters; + before, any parameter whose name or group contained words such as `scale` or + `background` (for example one of a material named "Background Material") was + counted as one. A parameter that is only named `thickness` or `roughness`, + outside a layer, is no longer labelled with its assembly's name. - Source cleanup : duplicated code and dead code removed (one shared plot-control group for the Experiment, Sample and Analysis pages, one definition of the experiment order), typed diff --git a/EasyReflectometryApp/Backends/Mock/Sample.qml b/EasyReflectometryApp/Backends/Mock/Sample.qml index 0efd17e2..03002cca 100644 --- a/EasyReflectometryApp/Backends/Mock/Sample.qml +++ b/EasyReflectometryApp/Backends/Mock/Sample.qml @@ -303,9 +303,12 @@ QtObject { // Layer magnetism (polarized analysis) readonly property bool magnetismSupported: true readonly property var layersMagnetism: [ - { 'label': 'label 1', 'magnetic': 'True', 'rho_m': '5.0', 'theta_m': '40.0', 'phi': '130.0', 'editable': 'True' }, - { 'label': 'label 2', 'magnetic': 'False', 'rho_m': '0.0', 'theta_m': '270.0', 'phi': '', 'editable': '' }, - { 'label': 'label 3', 'magnetic': 'False', 'rho_m': '0.0', 'theta_m': '270.0', 'phi': '', 'editable': '' }, + { 'label': 'label 1', 'magnetic': true, 'rho_m': '5.0', 'theta_m': '40.0', 'phi': '130.0', 'editable': true, + 'material_magnetic': false, 'rho_m_coupled': false }, + { 'label': 'label 2', 'magnetic': false, 'rho_m': '0.0', 'theta_m': '270.0', 'phi': '', 'editable': false, + 'material_magnetic': false, 'rho_m_coupled': false }, + { 'label': 'label 3', 'magnetic': false, 'rho_m': '0.0', 'theta_m': '270.0', 'phi': '', 'editable': false, + 'material_magnetic': false, 'rho_m_coupled': false }, ] function setLayerMagneticAtIndex(index, value) { console.debug(`setLayerMagneticAtIndex ${index} ${value}`) diff --git a/EasyReflectometryApp/Backends/Py/logic/assemblies.py b/EasyReflectometryApp/Backends/Py/logic/assemblies.py index c999d60f..2e84ac88 100644 --- a/EasyReflectometryApp/Backends/Py/logic/assemblies.py +++ b/EasyReflectometryApp/Backends/Py/logic/assemblies.py @@ -57,7 +57,7 @@ def type_at_current_index(self) -> str: return self._assemblies[self.index].type @property - def assemblies(self) -> list[dict[str, str]]: + def assemblies(self) -> list[dict[str, str | list[str]]]: rows = _from_assemblies_collection_to_list_of_dicts(self._assemblies) # An assembly is one object in every model sharing it (contrasts): editing it edits all. current = self._project_lib.current_model_index @@ -76,15 +76,19 @@ def assemblies_names(self) -> list[str]: def remove_at_index(self, index: int) -> None: self._assemblies.remove_assembly(index) - def add_new(self) -> None: + def add_new(self) -> bool: + """Insert an assembly with a Si layer; return whether the Si material had to be added to the project.""" previous_length = len(self._assemblies) target_index = self._target_insert_index(self.index, previous_length) existing_ids = {id(assembly) for assembly in self._assemblies} self._assemblies.add_assembly() new_index = self._move_new_assembly_into_position(existing_ids, target_index) + materials_before = len(self._project_lib._materials) + # The library adds Si to the project's materials when it is missing. index_si = self._project_lib.get_index_si() if new_index is not None: self._assemblies[new_index].layers[0].material = self._project_lib._materials[index_si] + return len(self._project_lib._materials) > materials_before def duplicate_selected(self) -> None: if not self._has_valid_assembly_index(self.index): @@ -176,6 +180,6 @@ def set_conformal_roughness(self, new_value: bool) -> bool: return False -def _from_assemblies_collection_to_list_of_dicts(assemblies_collection: Sample) -> list[dict[str, str]]: +def _from_assemblies_collection_to_list_of_dicts(assemblies_collection: Sample) -> list[dict[str, str | list[str]]]: # Repetitions and the surfactant flags have their own current-index properties. return [{'label': assembly.name, 'type': assembly.type} for assembly in assemblies_collection] diff --git a/EasyReflectometryApp/Backends/Py/logic/layers.py b/EasyReflectometryApp/Backends/Py/logic/layers.py index 8fcf35b5..3ce753b6 100644 --- a/EasyReflectometryApp/Backends/Py/logic/layers.py +++ b/EasyReflectometryApp/Backends/Py/logic/layers.py @@ -6,7 +6,6 @@ from easyreflectometry.sample import LayerAreaPerMolecule from easyreflectometry.sample import LayerCollection from easyreflectometry.sample import LayerMagnetism -from easyreflectometry.sample import Material from easyreflectometry.sample import Sample from easyreflectometry.sample.elements.layers.layer_magnetism import DEFAULTS as MAGNETISM_DEFAULTS @@ -68,15 +67,15 @@ def remove_at_index(self, index: int) -> None: def add_new(self) -> bool: """Append a Si layer; return whether the Si material had to be added to the project.""" - material_added = 'Si' not in [material.name for material in self._project_lib._materials] - if material_added: - self._project_lib._materials.add_material(Material(name='Si', sld=2.07, isld=0.0)) - index_si = [material.name for material in self._project_lib._materials].index('Si') + materials = self._project_lib._materials + count_before = len(materials) + # The library adds Si to the project's materials when it is missing. + si = materials[self._project_lib.get_index_si()] self._layers.add_layer() - self._layers[-1].material = self._project_lib._materials[index_si] + self._layers[-1].material = si # Set layer name based on material name - self._layers[-1].name = self._project_lib._materials[index_si].name + ' Layer' - return material_added + self._layers[-1].name = si.name + ' Layer' + return len(materials) > count_before def duplicate_selected(self) -> None: self._layers.duplicate_layer(self.index) @@ -220,19 +219,20 @@ def magnetism_supported(self) -> bool: return bool(self._project_lib.calculator_supports_magnetism) @property - def magnetism(self) -> list[dict[str, str]]: + def magnetism(self) -> list[dict[str, str | bool]]: """One row per layer of the current assembly, describing its magnetism. - ``magnetic`` is 'True'/'False'; ``rho_m``/``theta_m`` carry the defaults + ``magnetic`` is a bool; ``rho_m``/``theta_m`` carry the defaults of a fresh :class:`LayerMagnetism` for non-magnetic layers so the fields show what attaching magnetism would start from. ``phi`` is the direction the moment points, in degrees from the guide field - what the compass - draws - and ``editable`` whether that direction can be set by dragging - it (a constrained ``theta_m`` follows its expression, not the pointer). - Both are empty for a non-magnetic layer, which has no direction. - ``material_magnetic`` says whether the layer's material carries a - moment (a ``MaterialMagnetic``), and ``rho_m_coupled`` whether the - layer takes ``rho_m`` from it. + draws - and is empty for a non-magnetic layer, which has no direction. + ``editable`` says whether that direction can be set by dragging it (a + constrained ``theta_m`` follows its expression, not the pointer); it is + False for a non-magnetic layer. ``material_magnetic`` says whether the + layer's material carries a moment (a ``MaterialMagnetic``), and + ``rho_m_coupled`` whether the layer takes ``rho_m`` from it; both are + bools. """ rows = [] for layer in self._layers: @@ -240,13 +240,13 @@ def magnetism(self) -> list[dict[str, str]]: rows.append( { 'label': layer.name, - 'magnetic': str(magnetism is not None), + 'magnetic': magnetism is not None, 'rho_m': str(magnetism.rho_m.value if magnetism is not None else _DEFAULT_RHO_M), 'theta_m': str(magnetism.theta_m.value if magnetism is not None else _DEFAULT_THETA_M), 'phi': '' if magnetism is None else str(magnetic_vector_for_layer(magnetism)['phi']), - 'editable': '' if magnetism is None else str(magnetism.theta_m.independent), - 'material_magnetic': str(getattr(layer.material, 'rho_m', None) is not None), - 'rho_m_coupled': str(layer.rho_m_coupled), + 'editable': magnetism is not None and bool(magnetism.theta_m.independent), + 'material_magnetic': getattr(layer.material, 'rho_m', None) is not None, + 'rho_m_coupled': bool(layer.rho_m_coupled), } ) return rows diff --git a/EasyReflectometryApp/Backends/Py/logic/parameters.py b/EasyReflectometryApp/Backends/Py/logic/parameters.py index 964486e5..e8354595 100644 --- a/EasyReflectometryApp/Backends/Py/logic/parameters.py +++ b/EasyReflectometryApp/Backends/Py/logic/parameters.py @@ -315,6 +315,8 @@ def _get_dependency_expression(param: Parameter, paths: dict) -> str: def _is_layer_parameter(param: Parameter, path: list) -> bool: """Check if parameter is a layer's own thickness or roughness.""" + if len(path) < 2: + return False owner = path[-2] return isinstance(owner, Layer) and (param is owner.thickness or param is owner.roughness) @@ -324,7 +326,7 @@ def _is_magnetism_parameter(path: list) -> bool: Magnetism sits one level below the layer (Layer -> LayerMagnetism -> param), but belongs to the layer just as much, so it is named the same way. """ - return isinstance(path[-2], LayerMagnetism) + return len(path) >= 2 and isinstance(path[-2], LayerMagnetism) def _is_per_layer_parameter(param: Parameter, path: list) -> bool: """Per-layer parameters exist once per model and carry the model prefix.""" @@ -426,6 +428,10 @@ def _is_per_layer_parameter(param: Parameter, path: list) -> bool: 'detachable': len(shared_by) > 1 and is_layer_param and parameter.independent, 'followedBy': followed_by.get(parameter.unique_name, 0), 'object': parameter, # Direct reference to the Parameter object + # Decided under the first model that owns the parameter. That is + # enough while each model has its own scale and background objects + # (linked contrasts tie them with dependencies, not by sharing); + # if they are ever shared, test against every model in shared_by. 'experiment': _is_experiment_parameter(parameter, models[model_idx]), 'exclusive_with': _exclusive_partner(parameter, path), } diff --git a/EasyReflectometryApp/Backends/Py/sample.py b/EasyReflectometryApp/Backends/Py/sample.py index e35946c9..9781a1dd 100644 --- a/EasyReflectometryApp/Backends/Py/sample.py +++ b/EasyReflectometryApp/Backends/Py/sample.py @@ -477,7 +477,7 @@ def moveSelectedModelDown(self) -> None: # Assemblies # # # @Property('QVariantList', notify=assembliesTableChanged) - def assemblies(self) -> list[dict[str, str]]: + def assemblies(self) -> list[dict[str, str | list[str]]]: return self._assemblies_logic.assemblies @Property(int, notify=assembliesIndexChanged) @@ -593,7 +593,8 @@ def removeAssembly(self, index: int) -> None: @Slot() def addNewAssembly(self) -> None: - self._assemblies_logic.add_new() + if self._assemblies_logic.add_new(): + self.materialsTableChanged.emit() self._refreshCurrentAssemblySelectionState() self._project_logic._update_enablement_of_fixed_layers_for_model(self._models_logic.index) self.assembliesTableChanged.emit() @@ -861,7 +862,7 @@ def magnetismSupported(self) -> bool: return self._layers_logic.magnetism_supported @Property('QVariantList', notify=magnetismChanged) - def layersMagnetism(self) -> list[dict[str, str]]: + def layersMagnetism(self) -> list[dict[str, str | bool]]: """Per-layer magnetism of the current assembly, one row per layer.""" return self._layers_logic.magnetism diff --git a/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Magnetism.qml b/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Magnetism.qml index e154b80e..50bbebc5 100644 --- a/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Magnetism.qml +++ b/EasyReflectometryApp/Gui/Pages/Sample/Sidebar/Basic/Groups/Magnetism.qml @@ -100,7 +100,7 @@ EaElements.GroupBox { // are refreshed by separate signals, so a delegate can outlive // its row for one frame after a layer is removed. readonly property var rowData: Globals.BackendWrapper.sampleLayersMagnetism[index] ?? null - readonly property bool rowIsMagnetic: rowData !== null && rowData.magnetic === "True" + readonly property bool rowIsMagnetic: rowData !== null && rowData.magnetic === true EaComponents.TableViewLabel { color: EaStyle.Colors.themeForegroundMinor @@ -115,8 +115,8 @@ EaElements.GroupBox { EaComponents.TableViewTextInput { horizontalAlignment: Text.AlignHCenter // A layer coupled to its material shows the material's ρM. - enabled: rowIsMagnetic && rowData.rho_m_coupled !== "True" - ToolTip.text: rowData && rowData.rho_m_coupled === "True" ? + enabled: rowIsMagnetic && rowData.rho_m_coupled !== true + ToolTip.text: rowData && rowData.rho_m_coupled === true ? qsTr("Taken from the material's moment — untick 'ρM from material' below to edit") : '' text: rowData ? Number(rowData.rho_m).toFixed(3) : '--' onEditingFinished: Globals.BackendWrapper.sampleSetLayerRhoMAtIndex(index, text) @@ -155,14 +155,14 @@ EaElements.GroupBox { // holds, and the arrow the Structure view draws, side by side. Follows // the selection the table already writes - no second "focused row". Gui.MagnetizationAngleSlider { - visible: magnetismGroup.currentRow !== null && magnetismGroup.currentRow.magnetic === "True" + visible: magnetismGroup.currentRow !== null && magnetismGroup.currentRow.magnetic === true height: visible ? implicitHeight : 0 phi: visible ? Number(magnetismGroup.currentRow.phi) : 0 thetaM: visible ? Number(magnetismGroup.currentRow.theta_m) : 0 hasMoment: visible && Number(magnetismGroup.currentRow.rho_m) !== 0 // A constrained θM follows its expression, and a running fit owns // every parameter: in both cases the slider is read-only. - editable: visible && magnetismGroup.currentRow.editable === "True" + editable: visible && magnetismGroup.currentRow.editable === true && !Globals.BackendWrapper.analysisFittingRunning onThetaMRequested: (thetaM) => Globals.BackendWrapper.sampleSetLayerThetaMAtIndex( Globals.BackendWrapper.sampleCurrentLayerIndex, thetaM) @@ -172,10 +172,10 @@ EaElements.GroupBox { // material, see the Materials group); the layer keeps its own θM. EaElements.CheckBox { readonly property bool coupled: magnetismGroup.currentRow !== null - && magnetismGroup.currentRow.rho_m_coupled === "True" + && magnetismGroup.currentRow.rho_m_coupled === true visible: magnetismGroup.currentRow !== null - && magnetismGroup.currentRow.magnetic === "True" - && magnetismGroup.currentRow.material_magnetic === "True" + && magnetismGroup.currentRow.magnetic === true + && magnetismGroup.currentRow.material_magnetic === true topPadding: 0 text: qsTr("ρM from material") ToolTip.text: qsTr("Take ρM from the moment of this layer's magnetic material. Unticking keeps the current value.") diff --git a/tests/factories.py b/tests/factories.py index 5cc44da3..c714d42f 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -537,6 +537,9 @@ def get_index_air(self): return [material.name for material in self._materials].index('Air') def get_index_si(self): + # As in the library: Si is added to the materials when it is missing. + if 'Si' not in [material.name for material in self._materials]: + self._materials.add_material(FakeMaterial('Si', 2.07, 0.0)) return [material.name for material in self._materials].index('Si') def get_index_sio2(self): diff --git a/tests/test_logic_assemblies.py b/tests/test_logic_assemblies.py index f2fb001c..a61ec9ac 100644 --- a/tests/test_logic_assemblies.py +++ b/tests/test_logic_assemblies.py @@ -25,6 +25,20 @@ def test_from_assemblies_collection_to_list_of_dicts_has_the_same_keys_for_every ] +def test_assemblies_add_new_reports_whether_it_created_si(monkeypatch): + monkeypatch.setattr(assemblies_module, 'Multilayer', FakeMultilayer) + materials = make_material_collection(make_material('Air'), make_material('D2O')) + sample = make_sample(make_assembly(name='Existing', layers=[make_layer(material=materials[0])])) + project = make_project(materials=materials, models=make_model_collection(make_model(sample=sample))) + logic = assemblies_module.Assemblies(project) + + assert logic.add_new() is True + assert [material.name for material in project._materials] == ['Air', 'D2O', 'Si'] + + assert logic.add_new() is False + assert [material.name for material in project._materials] == ['Air', 'D2O', 'Si'] + + def test_assemblies_add_new_and_type_transitions(monkeypatch): monkeypatch.setattr(assemblies_module, 'Multilayer', FakeMultilayer) monkeypatch.setattr(assemblies_module, 'RepeatingMultilayer', FakeRepeatingMultilayer) @@ -35,7 +49,7 @@ def test_assemblies_add_new_and_type_transitions(monkeypatch): project = make_project(materials=materials, models=make_model_collection(model)) logic = assemblies_module.Assemblies(project) - logic.add_new() + assert logic.add_new() is False # Si exists and is reused assert logic._assemblies[-1].layers[0].material.name == 'Si' assert logic.set_name_at_current_index('Renamed Assembly') is True diff --git a/tests/test_logic_layers.py b/tests/test_logic_layers.py index fa35256e..afd15ab0 100644 --- a/tests/test_logic_layers.py +++ b/tests/test_logic_layers.py @@ -53,8 +53,7 @@ def test_from_layers_collection_to_list_of_dicts_gives_plain_layers_no_apm_place assert set(row) == {'label', 'roughness', 'thickness', 'material', 'thickness_enabled', 'roughness_enabled'} -def test_layers_add_new_creates_si_material_when_missing(monkeypatch): - monkeypatch.setattr(layers_module, 'Material', make_material) +def test_layers_add_new_creates_si_material_when_missing(): materials = make_material_collection(make_material('Air'), make_material('D2O')) sample = make_sample( make_assembly(name='Top'), @@ -76,8 +75,7 @@ def test_layers_add_new_creates_si_material_when_missing(monkeypatch): assert [material.name for material in project._materials] == ['Air', 'D2O', 'Si'] -def test_layers_move_duplicate_and_setters_update_current_layer(monkeypatch): - monkeypatch.setattr(layers_module, 'Material', make_material) +def test_layers_move_duplicate_and_setters_update_current_layer(): materials = make_material_collection(make_material('Air'), make_material('Si')) sample = make_sample( make_assembly(name='Top', layers=[make_layer(name='Top Layer', material=materials[0])]), @@ -117,7 +115,6 @@ def test_layers_move_duplicate_and_setters_update_current_layer(monkeypatch): def test_layers_index_based_setters_update_target_row_even_when_current_index_differs(monkeypatch): - monkeypatch.setattr(layers_module, 'Material', make_material) monkeypatch.setattr(layers_module, 'LayerAreaPerMolecule', FakeLayerAreaPerMolecule) air = make_material('Air') @@ -179,9 +176,7 @@ def test_layers_index_based_setters_update_target_row_even_when_current_index_di assert logic._layers[1].solvent.name == 'Air' -def test_layers_index_based_setters_ignore_invalid_indices(monkeypatch): - monkeypatch.setattr(layers_module, 'Material', make_material) - +def test_layers_index_based_setters_ignore_invalid_indices(): air = make_material('Air') si = make_material('Si') materials = make_material_collection(air, si) @@ -231,16 +226,16 @@ def test_magnetism_rows_report_the_drawn_direction(): plain, magnetic = logic.magnetism assert magnetic['phi'] == '130.0' # 40 deg is 130 deg from the guide field - assert magnetic['editable'] == 'True' + assert magnetic['editable'] is True # A non-magnetic layer has no direction and nothing to drag. - assert (plain['phi'], plain['editable']) == ('', '') + assert (plain['phi'], plain['editable']) == ('', False) def test_a_constrained_theta_m_is_reported_as_not_editable(): logic, _ = _magnetism_logic(theta_m=40.0, independent=False) # The slider reads this to go read-only rather than to write a refused value. - assert logic.magnetism[1]['editable'] == 'False' + assert logic.magnetism[1]['editable'] is False def test_a_negative_moment_points_the_arrow_the_other_way(): diff --git a/tests/test_logic_minimizers.py b/tests/test_logic_minimizers.py index 7c9ff8cf..033813f2 100644 --- a/tests/test_logic_minimizers.py +++ b/tests/test_logic_minimizers.py @@ -56,6 +56,30 @@ def test_selecting_writes_the_settings(): assert logic.set_minimizer_current_index(_index(logic, 'DFO_leastsq')) is False +def test_index_mapping_edges(): + project = _project() + logic = minimizers_module.Minimizers(project) + last = len(logic.minimizers_available()) - 1 + before = (project.fit_settings.mode, project.fit_settings.minimizer) + + # Outside the GUI list: refused, and the settings stay as they were. + for out_of_range in (-1, last + 1): + assert logic.set_minimizer_current_index(out_of_range) is False + assert (project.fit_settings.mode, project.fit_settings.minimizer) == before + + # The last entry is a real minimizer, one past the Bayesian choice at 0. + last_member = AvailableMinimizers[logic.minimizers_available()[last]] + assert logic.set_minimizer_current_index(last) is True + assert (project.fit_settings.mode, project.fit_settings.minimizer) == ('minimize', last_member) + assert logic.minimizer_current_index() == last + + # Index 1 is the first real minimizer. + first_member = AvailableMinimizers[logic.minimizers_available()[1]] + assert logic.set_minimizer_current_index(1) is True + assert project.fit_settings.minimizer is first_member + assert logic.minimizer_current_index() == 1 + + def test_generic_settings_work_before_any_fit_and_reset_to_engine_default(): project = _project() logic = minimizers_module.Minimizers(project) diff --git a/tests/test_logic_parameters.py b/tests/test_logic_parameters.py index 28163e44..aa423406 100644 --- a/tests/test_logic_parameters.py +++ b/tests/test_logic_parameters.py @@ -267,7 +267,13 @@ def test_parameters_filtering_metadata_and_current_parameter_updates(monkeypatch 'independent': True, 'kind': 'parameter', }, - {'alias': 'layer_thickness', 'displayName': 'Layer thickness', 'group': 'Layer', 'independent': False, 'kind': 'parameter'}, + { + 'alias': 'layer_thickness', + 'displayName': 'Layer thickness', + 'group': 'Layer', + 'independent': False, + 'kind': 'parameter', + }, ] logic.set_variability_filter_criteria('all') @@ -352,15 +358,31 @@ def test_from_parameters_to_list_of_dicts_handles_alias_edge_cases_and_fallbacks assert result[6]['enabled'] is True +def _filter_row(name, display_name, group, unique_name, alias, fit, experiment=False): + """A parameter row as ``all_parameters`` returns it, with only the keys the filters read.""" + return { + 'name': name, + 'display_name': display_name, + 'group': group, + 'unique_name': unique_name, + 'fit': fit, + 'enabled': True, + 'independent': True, + 'alias': alias, + 'object': object(), + 'experiment': experiment, + } + + def test_parameters_special_filters_and_invalid_variability_normalization(monkeypatch): project = make_project() logic = parameters_module.Parameters(project) mocked_parameters = [ - {'name': 'Cell length a', 'display_name': 'Cell length a', 'group': 'cell', 'unique_name': 'cell.a', 'fit': True, 'enabled': True, 'independent': True, 'alias': 'cell_a', 'object': object(), 'experiment': False}, - {'name': 'Atom site x', 'display_name': 'Atom site x', 'group': 'atom_site', 'unique_name': 'atom_site.x', 'fit': False, 'enabled': True, 'independent': True, 'alias': 'atom_site_x', 'object': object(), 'experiment': False}, - {'name': 'Biso', 'display_name': 'adp b_iso', 'group': 'thermal', 'unique_name': 'b_iso', 'fit': False, 'enabled': True, 'independent': True, 'alias': 'b_iso', 'object': object(), 'experiment': False}, - {'name': 'Instrument scale', 'display_name': 'Instrument scale', 'group': 'instrument', 'unique_name': 'scale', 'fit': True, 'enabled': True, 'independent': True, 'alias': 'scale', 'object': object(), 'experiment': True}, - {'name': 'Layer thickness', 'display_name': 'Layer thickness', 'group': 'layer', 'unique_name': 'layer.thickness', 'fit': True, 'enabled': True, 'independent': True, 'alias': 'thickness', 'object': object(), 'experiment': False}, + _filter_row('Cell length a', 'Cell length a', 'cell', 'cell.a', 'cell_a', fit=True), + _filter_row('Atom site x', 'Atom site x', 'atom_site', 'atom_site.x', 'atom_site_x', fit=False), + _filter_row('Biso', 'adp b_iso', 'thermal', 'b_iso', 'b_iso', fit=False), + _filter_row('Instrument scale', 'Instrument scale', 'instrument', 'scale', 'scale', fit=True, experiment=True), + _filter_row('Layer thickness', 'Layer thickness', 'layer', 'layer.thickness', 'thickness', fit=True), ] monkeypatch.setattr(logic, 'all_parameters', lambda: mocked_parameters) @@ -368,7 +390,9 @@ def test_parameters_special_filters_and_invalid_variability_normalization(monkey assert logic.variability_filter_criteria == 'all' logic.set_name_filter_criteria('model') - assert [entry['display_name'] for entry in logic.parameters] == ['Cell length a', 'Atom site x', 'adp b_iso', 'Layer thickness'] + assert [entry['display_name'] for entry in logic.parameters] == [ + 'Cell length a', 'Atom site x', 'adp b_iso', 'Layer thickness', + ] logic.set_name_filter_criteria('experiment') assert [entry['display_name'] for entry in logic.parameters] == ['Instrument scale'] diff --git a/tests/test_magnetic_material_and_field.py b/tests/test_magnetic_material_and_field.py index ece7a7be..562ffd05 100644 --- a/tests/test_magnetic_material_and_field.py +++ b/tests/test_magnetic_material_and_field.py @@ -43,13 +43,13 @@ class TestCoupling: def test_rows_report_the_material_and_the_coupling(self, project): logic = Layers(project) row = logic.magnetism[0] - assert (row['material_magnetic'], row['rho_m_coupled']) == ('True', 'False') + assert (row['material_magnetic'], row['rho_m_coupled']) == (True, False) assert logic.set_rho_m_coupled_at_index(0, True) is True assert logic.set_rho_m_coupled_at_index(0, True) is False row = logic.magnetism[0] - assert row['rho_m_coupled'] == 'True' + assert row['rho_m_coupled'] is True assert float(row['rho_m']) == pytest.approx(FE_RHO_M, abs=1e-3) def test_coupled_rho_m_cannot_be_typed(self, project): diff --git a/tests/test_magnetic_orso_load.py b/tests/test_magnetic_orso_load.py index d37065e8..eff88b79 100644 --- a/tests/test_magnetic_orso_load.py +++ b/tests/test_magnetic_orso_load.py @@ -83,7 +83,7 @@ def test_a_moment_gives_a_magnetic_material_and_a_coupled_layer(self, backend, t assert material_row['magnetic'] is True assert float(material_row['magnetic_moment']) == pytest.approx(2.2) row = _film_row(backend) - assert (row['magnetic'], row['material_magnetic'], row['rho_m_coupled']) == ('True', 'True', 'True') + assert (row['magnetic'], row['material_magnetic'], row['rho_m_coupled']) == (True, True, True) assert float(row['rho_m']) == pytest.approx(FE_RHO_M, abs=1e-3) assert warnings_received == [] diff --git a/tests/test_polarized_analysis.py b/tests/test_polarized_analysis.py index bfa758f9..5307487f 100644 --- a/tests/test_polarized_analysis.py +++ b/tests/test_polarized_analysis.py @@ -54,7 +54,7 @@ def test_rows_describe_every_layer_of_the_assembly(self): rows = logic.magnetism assert len(rows) == len(logic.layers) - assert rows[0]['magnetic'] == 'False' + assert rows[0]['magnetic'] is False # A non-magnetic layer previews what attaching magnetism would create. assert float(rows[0]['rho_m']) == 0.0 assert float(rows[0]['theta_m']) == 270.0 @@ -65,7 +65,7 @@ def test_toggle_on_attaches_magnetism_and_enables_the_calculator(self): assert logic.set_magnetic_at_index(0, True) is True - assert logic.magnetism[0]['magnetic'] == 'True' + assert logic.magnetism[0]['magnetic'] is True assert project.models[0].has_magnetism is True assert project._calculator().include_magnetism is True @@ -76,7 +76,7 @@ def test_toggle_off_removes_magnetism_and_disables_the_calculator(self): assert logic.set_magnetic_at_index(0, False) is True - assert logic.magnetism[0]['magnetic'] == 'False' + assert logic.magnetism[0]['magnetic'] is False assert project.models[0].has_magnetism is False assert project._calculator().include_magnetism is False @@ -375,7 +375,7 @@ def test_slots_emit_and_report_failure(self, qcore_application): backend.setLayerThetaMAtIndex(0, 45.0) assert backend.magnetismSupported is True - assert backend.layersMagnetism[0]['magnetic'] == 'True' + assert backend.layersMagnetism[0]['magnetic'] is True assert float(backend.layersMagnetism[0]['rho_m']) == 3.0 assert len(changed) == 3 @@ -395,7 +395,7 @@ def test_unsupported_calculator_offers_the_engine_that_can(self, qcore_applicati assert backend.magnetismSupported is False assert requests == [(0, 'refl1d')] assert failures == [] - assert backend.layersMagnetism[0]['magnetic'] == 'False' + assert backend.layersMagnetism[0]['magnetic'] is False def test_confirmed_switch_changes_the_engine_and_attaches_magnetism(self, qcore_application): from EasyReflectometryApp.Backends.Py.sample import Sample @@ -408,7 +408,7 @@ def test_confirmed_switch_changes_the_engine_and_attaches_magnetism(self, qcore_ assert backend.magnetismSupported is True assert backend.calculationEngines[backend.calculationEngineIndex] == 'refl1d' - assert backend.layersMagnetism[0]['magnetic'] == 'True' + assert backend.layersMagnetism[0]['magnetic'] is True assert engine_changes == [True] def test_engine_that_cannot_carry_the_magnetism_is_refused(self, qcore_application): @@ -425,7 +425,7 @@ def test_engine_that_cannot_carry_the_magnetism_is_refused(self, qcore_applicati # The refusal goes to the engine selector's dialog, not the log. assert len(rejections) == 1 and 'refnx' in rejections[0] assert backend.calculationEngines[backend.calculationEngineIndex] == 'refl1d' - assert backend.layersMagnetism[0]['magnetic'] == 'True' + assert backend.layersMagnetism[0]['magnetic'] is True class _StubMinimizers: diff --git a/tests/test_py_sample_fixed_layers.py b/tests/test_py_sample_fixed_layers.py index 44e371f2..544f056d 100644 --- a/tests/test_py_sample_fixed_layers.py +++ b/tests/test_py_sample_fixed_layers.py @@ -168,6 +168,21 @@ def test_added_layer_announces_the_si_material_only_when_it_creates_it(project_a assert 'Si' in [material.name for material in project._materials] +def test_added_assembly_announces_the_si_material_only_when_it_creates_it(project_and_backend): + project, backend = project_and_backend + backend.setCurrentAssemblyIndex(1) + emitted = [] + backend.materialsTableChanged.connect(lambda: emitted.append(True)) + + backend.addNewAssembly() # the default project already has Si + assert emitted == [] + + next(material for material in project._materials if material.name == 'Si').name = 'Silicon' + backend.addNewAssembly() + assert emitted == [True] + assert 'Si' in [material.name for material in project._materials] + + def test_middle_assembly_table_is_fully_editable(project_and_backend): project, backend = project_and_backend backend.setCurrentAssemblyIndex(1) diff --git a/tests/test_qml_magnetization_slider.py b/tests/test_qml_magnetization_slider.py index 3ff20854..57c6382f 100644 --- a/tests/test_qml_magnetization_slider.py +++ b/tests/test_qml_magnetization_slider.py @@ -62,7 +62,7 @@ def test_the_slider_follows_the_selection_the_table_already_writes(): assert 'Globals.BackendWrapper.sampleLayersMagnetism[Globals.BackendWrapper.sampleCurrentLayerIndex]' in group_qml assert 'Gui.MagnetizationAngleSlider {' in group_qml # Gone entirely - not just greyed out - for a layer with no magnetism. - assert 'visible: magnetismGroup.currentRow !== null && magnetismGroup.currentRow.magnetic === "True"' in group_qml + assert 'visible: magnetismGroup.currentRow !== null && magnetismGroup.currentRow.magnetic === true' in group_qml assert 'height: visible ? implicitHeight : 0' in group_qml @@ -71,7 +71,7 @@ def test_the_group_writes_through_the_backend_and_refuses_when_it_must_not(): # The same call the theta_m column makes: one write path for one parameter. assert 'sampleSetLayerThetaMAtIndex' in group_qml - assert 'magnetismGroup.currentRow.editable === "True"' in group_qml + assert 'magnetismGroup.currentRow.editable === true' in group_qml assert '!Globals.BackendWrapper.analysisFittingRunning' in group_qml @@ -82,7 +82,7 @@ def test_the_backend_contract_is_mirrored_in_the_wrapper_and_the_mock(): assert 'sampleSetLayerThetaMAtIndex' in wrapper assert 'function setLayerThetaMAtIndex(index, value)' in mock assert "'phi': '130.0'" in mock - assert "'editable': 'True'" in mock + assert "'editable': true" in mock def test_the_dial_is_gone():