diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d5abc83e2..dae109fd6 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -173,6 +173,15 @@ ## Workflow +- Use a two-phase workflow for all non-trivial changes: + - **Phase 1 — Implementation:** implement the change (source code, + docs, architecture updates). Do not create new tests or run existing + tests. Present the implementation for review and iterate until + approved. + - **Phase 2 — Verification:** once the implementation is approved, add + or update tests, then run linting (`pixi run fix`, `pixi run check`) + and all test suites (`pixi run unit-tests`, + `pixi run integration-tests`, `pixi run script-tests`). - All open issues, design questions, and planned improvements are tracked in `docs/architecture/issues_open.md`, ordered by priority. When an issue is fully implemented, move it from that file to @@ -180,9 +189,12 @@ architecture, update the relevant sections of `docs/architecture/architecture.md`. - After changes, run linting and formatting fixes with `pixi run fix`. - Do not check what was auto-fixed, just accept the fixes and move on. - Then, run linting and formatting checks with `pixi run check` and - address any remaining issues until the code is clean. + This also regenerates `docs/architecture/package-structure-full.md` + and `docs/architecture/package-structure-short.md` automatically — do + not edit those files by hand. Do not check what was auto-fixed, just + accept the fixes and move on. Then, run linting and formatting checks + with `pixi run check` and address any remaining issues until the code + is clean. - After changes, run unit tests with `pixi run unit-tests`. - After changes, run integration tests with `pixi run integration-tests`. diff --git a/docs/architecture/architecture.md b/docs/architecture/architecture.md index a3002ab12..1615307d1 100644 --- a/docs/architecture/architecture.md +++ b/docs/architecture/architecture.md @@ -270,9 +270,6 @@ experiment.data # CategoryCollection experiment.background_type = 'chebyshev' # triggers BackgroundFactory.create(...) experiment.peak_profile_type = 'thompson-cox-hastings' # triggers PeakFactory.create(...) experiment.extinction_type = 'becker-coppens' # triggers ExtinctionFactory.create(...) -experiment.linked_crystal_type = 'default' # triggers LinkedCrystalFactory.create(...) -experiment.excluded_regions_type = 'default' # triggers ExcludedRegionsFactory.create(...) -experiment.linked_phases_type = 'default' # triggers LinkedPhasesFactory.create(...) ``` **Type switching pattern:** `expt.background_type = 'chebyshev'` rather @@ -389,26 +386,26 @@ from .line_segment import LineSegmentBackground ### 5.5 All Factories -| Factory | Domain | Tags resolve to | -| ---------------------------- | ---------------------- | ------------------------------------------------------------ | -| `BackgroundFactory` | Background categories | `LineSegmentBackground`, `ChebyshevPolynomialBackground` | -| `PeakFactory` | Peak profiles | `CwlPseudoVoigt`, `TofJorgensen`, `TofJorgensenVonDreele`, … | -| `InstrumentFactory` | Instruments | `CwlPdInstrument`, `TofPdInstrument`, … | -| `DataFactory` | Data collections | `PdCwlData`, `PdTofData`, `ReflnData`, `TotalData` | -| `ExtinctionFactory` | Extinction models | `BeckerCoppensExtinction` | -| `LinkedCrystalFactory` | Linked-crystal refs | `LinkedCrystal` | -| `ExcludedRegionsFactory` | Excluded regions | `ExcludedRegions` | -| `LinkedPhasesFactory` | Linked phases | `LinkedPhases` | -| `ExperimentTypeFactory` | Experiment descriptors | `ExperimentType` | -| `CellFactory` | Unit cells | `Cell` | -| `SpaceGroupFactory` | Space groups | `SpaceGroup` | -| `AtomSitesFactory` | Atom sites | `AtomSites` | -| `AliasesFactory` | Parameter aliases | `Aliases` | -| `ConstraintsFactory` | Parameter constraints | `Constraints` | -| `FitModeFactory` | Fit-mode category | `FitMode` | -| `JointFitExperimentsFactory` | Joint-fit weights | `JointFitExperiments` | -| `CalculatorFactory` | Calculation engines | `CryspyCalculator`, `CrysfmlCalculator`, `PdffitCalculator` | -| `MinimizerFactory` | Minimisers | `LmfitMinimizer`, `DfolsMinimizer`, … | +| Factory | Domain | Tags resolve to | +| ---------------------------- | ---------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `BackgroundFactory` | Background categories | `LineSegmentBackground`, `ChebyshevPolynomialBackground` | +| `PeakFactory` | Peak profiles | `CwlPseudoVoigt`, `TofJorgensen`, `TofJorgensenVonDreele`, … | +| `InstrumentFactory` | Instruments | `CwlPdInstrument`, `TofPdInstrument`, … | +| `DataFactory` | Data collections | `PdCwlData`, `PdTofData`, `ReflnData`, `TotalData` | +| `ExtinctionFactory` | Extinction models | `BeckerCoppensExtinction` | +| `LinkedCrystalFactory` | Linked-crystal refs | `LinkedCrystal` | +| `ExcludedRegionsFactory` | Excluded regions | `ExcludedRegions` | +| `LinkedPhasesFactory` | Linked phases | `LinkedPhases` | +| `ExperimentTypeFactory` | Experiment descriptors | `ExperimentType` | +| `CellFactory` | Unit cells | `Cell` | +| `SpaceGroupFactory` | Space groups | `SpaceGroup` | +| `AtomSitesFactory` | Atom sites | `AtomSites` | +| `AliasesFactory` | Parameter aliases | `Aliases` | +| `ConstraintsFactory` | Parameter constraints | `Constraints` | +| `FitModeFactory` | Fit-mode category | `FitMode` | +| `JointFitExperimentsFactory` | Joint-fit weights | `JointFitExperiments` | +| `CalculatorFactory` | Calculation engines | `CryspyCalculator`, `CrysfmlCalculator`, `PdffitCalculator` | +| `MinimizerFactory` | Minimisers | `LmfitMinimizer`, `LmfitLeastsqMinimizer`, `LmfitLeastSquaresMinimizer`, `DfolsMinimizer`, `BumpsMinimizer`, `BumpsLmMinimizer`, `BumpsAmoebaMinimizer`, `BumpsDEMinimizer` | > **Note:** `ExperimentFactory` and `StructureFactory` are _builder_ > factories with `from_cif_path`, `from_cif_str`, `from_data_path`, and @@ -509,16 +506,16 @@ Tags are the user-facing identifiers for selecting types. They must be: **Minimizer tags** -| Tag | Class | -| ----------------------- | ----------------------------------------- | -| `lmfit` | `LmfitMinimizer` | -| `lmfit (leastsq)` | `LmfitMinimizer` (method=`leastsq`) | -| `lmfit (least_squares)` | `LmfitMinimizer` (method=`least_squares`) | -| `dfols` | `DfolsMinimizer` | - -> **Note:** minimizer variant tags (`lmfit (leastsq)`, -> `lmfit (least_squares)`) are planned but not yet re-implemented after -> the `FactoryBase` migration. See `issues_open.md` for details. +| Tag | Class | +| ----------------------- | ---------------------------- | +| `lmfit` | `LmfitMinimizer` | +| `lmfit (leastsq)` | `LmfitLeastsqMinimizer` | +| `lmfit (least_squares)` | `LmfitLeastSquaresMinimizer` | +| `dfols` | `DfolsMinimizer` | +| `bumps` | `BumpsMinimizer` | +| `bumps (lm)` | `BumpsLmMinimizer` | +| `bumps (amoeba)` | `BumpsAmoebaMinimizer` | +| `bumps (de)` | `BumpsDEMinimizer` | ### 5.7 Metadata Classification — Which Classes Get What @@ -605,6 +602,7 @@ line-segment points. | `PdffitCalculator` | `CalculatorFactory` | (same) | | `LmfitMinimizer` | `MinimizerFactory` | `type_info` only | | `DfolsMinimizer` | `MinimizerFactory` | (same) | +| `BumpsMinimizer` | `MinimizerFactory` | (same) | | `BraggPdExperiment` | `ExperimentFactory` | `type_info` + `compatibility` (no `calculator_support`) | | `TotalPdExperiment` | `ExperimentFactory` | (same) | | `CwlScExperiment` | `ExperimentFactory` | (same) | @@ -636,7 +634,7 @@ The experiment exposes the standard switchable-category API: ### 6.2 Minimiser The minimiser drives the optimisation loop. `MinimizerFactory` creates -instances by tag (e.g. `'lmfit'`, `'dfols'`). +instances by tag (e.g. `'lmfit'`, `'dfols'`, `'bumps'`). ### 6.3 Fitter @@ -658,14 +656,19 @@ workflow: - Fit mode: `fit_mode` (`CategoryItem` with a `mode` descriptor validated by `FitModeEnum`); `'single'` fits each experiment independently, `'joint'` fits all simultaneously with weights from - `joint_fit_experiments`. + `joint_fit_experiments`, `'sequential'` records that sequential + fitting was used. `show_supported_fit_mode_types()` filters by + experiment count (≤1 → only `single`; >1 → all three). + `show_current_fit_mode_type()` prints the current mode. - Joint-fit weights: `joint_fit_experiments` (`CategoryCollection` of per-experiment weight entries); sibling of `fit_mode`, not a child. - Parameter tables: `show_all_params()`, `show_fittable_params()`, `show_free_params()`, `how_to_access_parameters()` -- Fitting: `fit()`, `show_fit_results()` -- Aliases and constraints (switchable categories with `aliases_type`, - `constraints_type`, `fit_mode_type`, `joint_fit_experiments_type`) +- Fitting: `fit()` dispatches single/joint; `fit_sequential()` handles + sequential mode (sets `fit_mode` to `'sequential'` internally). + `display.fit_results()` shows results. +- Aliases and constraints (single-type categories; no public `_type` + getter or setter) --- @@ -955,8 +958,11 @@ and simplifies maintenance. Categories whose concrete implementation can be swapped at runtime (background, peak profile, etc.) are called **switchable categories**. **Every category must be factory-based** — even if only one -implementation exists today. This ensures a uniform API, consistent -discoverability, and makes adding a second implementation trivial. +implementation exists today. This ensures uniform construction, +consistent metadata, and makes adding a second implementation trivial. + +For categories with **multiple implementations** (multi-type), the owner +exposes the full switchable API: | Facet | Naming pattern | Example | | --------------- | -------------------------------------------- | ------------------------------------------------ | @@ -965,15 +971,33 @@ discoverability, and makes adding a second implementation trivial. | Show supported | `show_supported__types()` | `expt.show_supported_background_types()` | | Show current | `show_current__type()` | `expt.show_current_peak_profile_type()` | -The convention applies universally: +Multi-type categories: - **Experiment:** `calculator_type`, `background_type`, - `peak_profile_type`, `extinction_type`, `linked_crystal_type`, - `excluded_regions_type`, `linked_phases_type`, `instrument_type`, - `data_type`. -- **Structure:** `cell_type`, `space_group_type`, `atom_sites_type`. -- **Analysis:** `aliases_type`, `constraints_type`, `fit_mode_type`, - `joint_fit_experiments_type`. + `peak_profile_type`, `extinction_type`. + +Categories that are **fixed at creation** (determined by the experiment +type and never changed) expose only a read-only `` property +with no `_type` getter, setter, or show methods: + +- **Experiment:** `instrument`, `data`. + +For categories with **only one implementation** (single-type), the +`_type` getter, setter, and show methods are omitted from the public API +to avoid clutter. The factory and `_type` attribute still exist +internally for consistency and future extensibility. + +Single-type categories (no public `_type` property): + +- **Experiment:** `diffrn`, `linked_crystal`, `excluded_regions`, + `linked_phases`. +- **Structure:** `cell`, `space_group`, `atom_sites`. +- **Analysis:** `aliases`, `constraints`. + +`fit_mode` has show methods (`show_supported_fit_mode_types()`, +`show_current_fit_mode_type()`) but no public `_type` getter or setter +because it has only one factory implementation. The mode is changed via +the `fit_mode.mode` descriptor directly. **Design decisions:** @@ -998,18 +1022,6 @@ expt.show_supported_peak_profile_types() expt.show_supported_background_types() expt.show_supported_calculator_types() expt.show_supported_extinction_types() -expt.show_supported_linked_crystal_types() -expt.show_supported_excluded_regions_types() -expt.show_supported_linked_phases_types() -expt.show_supported_instrument_types() -expt.show_supported_data_types() -struct.show_supported_cell_types() -struct.show_supported_space_group_types() -struct.show_supported_atom_sites_types() -project.analysis.show_supported_aliases_types() -project.analysis.show_supported_constraints_types() -project.analysis.show_supported_fit_mode_types() -project.analysis.show_supported_joint_fit_experiments_types() project.analysis.show_available_minimizers() ``` @@ -1026,7 +1038,7 @@ class. This applies to: - Factory tags (§5.6) — e.g. `PeakProfileTypeEnum`, `CalculatorEnum`. - Experiment-axis values — e.g. `SampleFormEnum`, `BeamModeEnum`. - Category descriptors with enumerated choices — e.g. fit mode - (`FitModeEnum.SINGLE`, `FitModeEnum.JOINT`). + (`FitModeEnum.SINGLE`, `FitModeEnum.JOINT`, `FitModeEnum.SEQUENTIAL`). The enum serves as the **single source of truth** for valid values, their user-facing string representations, and their descriptions. diff --git a/docs/architecture/issues_closed.md b/docs/architecture/issues_closed.md index 9a73c48c9..b98b9544f 100644 --- a/docs/architecture/issues_closed.md +++ b/docs/architecture/issues_closed.md @@ -4,6 +4,16 @@ Issues that have been fully resolved. Kept for historical reference. --- +## Restore Minimiser Variant Support + +Used thin subclasses (approach A) to restore lmfit algorithm variants. +`LmfitLeastsqMinimizer` and `LmfitLeastSquaresMinimizer` extend +`LmfitMinimizer`, each with its own `TypeInfo` tag. Added +`MinimizerTypeEnum` for all minimizer tags. No `FactoryBase` changes +needed — one class per tag. + +--- + ## Implement `Project.load()` `Project.load(dir_path)` classmethod reads `project.cif`, @@ -89,3 +99,27 @@ output. Unified `plot_param_series()` to read from CSV. Added also writes CSV. Prerequisites included: CIF truncation fix, CIF round-trip verification, `analysis.cif` moved into `analysis/` directory, `extract_data_paths_from_zip` destination parameter. + +--- + +## Make `data_type` Read-Only on Experiments + +Removed the `data_type` setter, `show_supported_data_types()`, and +`show_current_data_type()` from both `ScExperimentBase` and +`PdExperimentBase`. The data collection type is now fixed at experiment +creation (resolved from `DataFactory.default_tag(...)` using the +experiment's axes), like experiment type itself. This prevents switching +to an incompatible data class and silently discarding loaded data. + +--- + +## 78. Add `SEQUENTIAL` to `FitModeEnum` and Show Methods to Analysis + +Added `SEQUENTIAL = 'sequential'` to `FitModeEnum`. `fit_sequential()` +now sets `fit_mode.mode = 'sequential'` internally so the mode is +persisted in CIF. Added `show_supported_fit_mode_types()` (filters by +experiment count: ≤1 → only `single`; >1 → all three) and +`show_current_fit_mode_type()` on `Analysis`. If `fit()` is called while +mode is `'sequential'`, it logs an error directing the user to +`fit_sequential()`. Promoted `fit_mode` from a pure single-type category +to one with show methods. diff --git a/docs/architecture/issues_open.md b/docs/architecture/issues_open.md index c7cbaf180..799a2603e 100644 --- a/docs/architecture/issues_open.md +++ b/docs/architecture/issues_open.md @@ -10,43 +10,6 @@ needed. --- -## 2. 🟡 Restore Minimiser Variant Support - -**Type:** Feature loss + Design limitation - -After the `FactoryBase` migration only `'lmfit'` and `'dfols'` remain as -registered tags. The ability to select a specific lmfit algorithm (e.g. -`'lmfit (leastsq)'`, `'lmfit (least_squares)'`) raises a `ValueError`. - -The root cause is that `FactoryBase` assumes one class ↔ one tag; -registering the same class twice with different constructor arguments is -not supported. - -**Fix:** decide on an approach (thin subclasses, extended registry, or -two-level selection) and implement. Thin subclasses is the quickest. - -**Planned tags:** - -| Tag | Description | -| ----------------------- | ------------------------------------------------------------------------ | -| `lmfit` | LMFIT library using the default Levenberg-Marquardt least squares method | -| `lmfit (leastsq)` | LMFIT library with Levenberg-Marquardt least squares method | -| `lmfit (least_squares)` | LMFIT library with SciPy's trust region reflective algorithm | -| `dfols` | DFO-LS library for derivative-free least-squares optimization | - -**Trade-offs:** - -| Approach | Pros | Cons | -| -------------------------------------------------------- | ---------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- | -| **A. Thin subclasses** (one per variant) | Works today; each variant gets full metadata; no `FactoryBase` changes | Class proliferation; boilerplate | -| **B. Extend registry to store `(class, kwargs)` tuples** | No extra classes; factory handles variants natively | `_supported_map` changes shape; `TypeInfo` moves from class attribute to registration-time data | -| **C. Two-level selection** (`engine` + `algorithm`) | Clean separation; engine maps to class, algorithm is a constructor arg | More complex API (`current_minimizer = ('lmfit', 'least_squares')`); needs new `FactoryBase` protocol | - -**Depends on:** nothing (standalone, but should be decided before more -factories adopt variants). - ---- - ## 3. 🟡 Rebuild Joint-Fit Weights on Every Fit **Type:** Fragility @@ -81,31 +44,6 @@ CIF serialisation. --- -## 6. 🔴 Restrict `data_type` Switching to Compatible Types and Preserve Data Safety - -**Type:** Correctness + Data safety - -`Experiment.data_type` currently validates against all registered data -tags rather than only those compatible with the experiment's -`sample_form` / `scattering_type` / `beam_mode`. This allows users to -switch an experiment to an incompatible data collection class. The -setter also replaces the existing data object with a fresh empty -instance, discarding loaded data without warning. - -**Why high:** the current API can create internally inconsistent -experiments and silently lose measured data, which is especially -dangerous for notebook and tutorial workflows. - -**Fix:** filter supported data types through -`DataFactory.supported_for(...)` using the current experiment context, -and warn or block when a switch would discard existing data. If runtime -data-type switching is not a real user need, consider making `data` -effectively fixed after experiment creation. - -**Depends on:** nothing. - ---- - ## 8. 🟡 Add Explicit `create()` Signatures on Collections **Type:** API safety @@ -261,19 +199,1326 @@ re-derivable default. --- +## 17. 🟢 Use PDF-Specific CIF Names for Total Scattering + +**Type:** Naming + +The `TotalPdDataPoint` class reuses Bragg powder CIF tag names (e.g. +`_pd_data.point_id`, `_pd_proc.r`, `_pd_meas.intensity_total`) as +placeholders. These should be replaced with proper total-scattering / +PDF-specific CIF names. + +**TODOs:** + +- [total_pd.py](src/easydiffraction/datablocks/experiment/categories/data/total_pd.py#L48) + — `_pd_data.point_id` +- [total_pd.py](src/easydiffraction/datablocks/experiment/categories/data/total_pd.py#L62) + — `_pd_proc.r` +- [total_pd.py](src/easydiffraction/datablocks/experiment/categories/data/total_pd.py#L74) + — `_pd_meas.intensity_total` +- [total_pd.py](src/easydiffraction/datablocks/experiment/categories/data/total_pd.py#L87) + — `_pd_meas.intensity_total_su` +- [total_pd.py](src/easydiffraction/datablocks/experiment/categories/data/total_pd.py#L99) + — `_pd_calc.intensity_total` +- [total_pd.py](src/easydiffraction/datablocks/experiment/categories/data/total_pd.py#L112) + — `_pd_data.refinement_status` + +**Depends on:** nothing. + +--- + +## 18. 🟢 Move CIF v2→v1 Conversion Out of Calculator + +**Type:** Maintainability + +`PdffitCalculator.calculate_pattern` contains inline CIF v2→v1 +conversion (dot-to-underscore rewriting). This should live in a shared +`io` module. + +**TODOs:** + +- [pdffit.py](src/easydiffraction/analysis/calculators/pdffit.py#L118) + +**Depends on:** nothing. + +--- + +## 19. 🟢 Add Debug-Mode Logging for Calculator Imports + +**Type:** Diagnostics + +Several calculator modules have commented-out print statements for +import success/failure. These should be wired into the logging system +under a debug level. + +**TODOs:** + +- [pdffit.py](src/easydiffraction/analysis/calculators/pdffit.py#L34) +- [pdffit.py](src/easydiffraction/analysis/calculators/pdffit.py#L37) +- [crysfml.py](src/easydiffraction/analysis/calculators/crysfml.py#L19) +- [crysfml.py](src/easydiffraction/analysis/calculators/crysfml.py#L23) +- [cryspy.py](src/easydiffraction/analysis/calculators/cryspy.py#L25) +- [cryspy.py](src/easydiffraction/analysis/calculators/cryspy.py#L28) + +**Depends on:** nothing. + +--- + +## 20. 🟢 Redirect or Suppress CrysPy stderr Warnings + +**Type:** UX + +CrysPy emits warnings to stderr during pattern calculation. The code has +TODO markers to redirect these. + +**TODOs:** + +- [cryspy.py](src/easydiffraction/analysis/calculators/cryspy.py#L112) +- [cryspy.py](src/easydiffraction/analysis/calculators/cryspy.py#L184) + +**Depends on:** nothing. + +--- + +## 21. 🟡 Clarify CrysPy TOF Background CIF Tag Names + +**Type:** Correctness / Naming + +The CrysPy calculator uses TOF background CIF tags +(`_tof_backgroundpoint_time`, `_tof_backgroundpoint_intensity`) and +hardcoded `0.0` intensity values marked with `TODO: !!!!????`. The +mapping and the hardcoded defaults need verification. + +**TODOs:** + +- [cryspy.py](src/easydiffraction/analysis/calculators/cryspy.py#L734) +- [cryspy.py](src/easydiffraction/analysis/calculators/cryspy.py#L735) +- [cryspy.py](src/easydiffraction/analysis/calculators/cryspy.py#L738) +- [cryspy.py](src/easydiffraction/analysis/calculators/cryspy.py#L739) + +**Depends on:** nothing. + +--- + +## 22. 🟢 Check CrysPy Single-Crystal Instrument Mapping + +**Type:** Correctness + +`_cif_instrument_section` uses an empty `instrument_mapping` dict for +single crystal and a `TODO: Check this mapping!` marker. + +**TODOs:** + +- [cryspy.py](src/easydiffraction/analysis/calculators/cryspy.py#L506) + +**Depends on:** nothing. + +--- + +## 23. 🟢 Investigate PyCrysFML Pattern Length Discrepancy + +**Type:** Correctness + +CrysFML calculator adjusts pattern length post-calculation with a TODO +asking to investigate the origin of the off-by-one discrepancy. The same +epsilon workaround appears in the dict builder. + +**TODOs:** + +- [crysfml.py](src/easydiffraction/analysis/calculators/crysfml.py#L124) +- [crysfml.py](src/easydiffraction/analysis/calculators/crysfml.py#L253) + +**Depends on:** nothing. + +--- + +## 24. 🟢 Process Default Values on Experiment Creation + +**Type:** Design + +Default instrument/peak values for the CrysFML dict are filled in at +calculation time with inline fallbacks rather than being set at +experiment creation. + +**TODOs:** + +- [crysfml.py](src/easydiffraction/analysis/calculators/crysfml.py#L233) + +**Depends on:** nothing. + +--- + +## 25. 🟡 Refactor Data `_update` Methods (Split and Unify) + +**Type:** Maintainability + +Multiple `_update` helpers in Bragg PD, Bragg SC, and Total PD data +classes have `TODO: split into multiple methods` or +`TODO: refactor _get_valid_linked_phases` markers. The update logic +should be decomposed and the `_get_valid_linked_phases` responsibility +should be narrowed. The Total PD and Bragg PD classes should also adapt +the pattern from `bragg_sc.py`. + +**TODOs:** + +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L386) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L389) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L506) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L585) +- [bragg_sc.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_sc.py#L271) +- [total_pd.py](src/easydiffraction/datablocks/experiment/categories/data/total_pd.py#L254) +- [total_pd.py](src/easydiffraction/datablocks/experiment/categories/data/total_pd.py#L257) +- [total_pd.py](src/easydiffraction/datablocks/experiment/categories/data/total_pd.py#L349) + +**Depends on:** nothing. + +--- + +## 26. 🟢 Clarify `dtype` Usage in Data Point Arrays + +**Type:** Cleanup + +Many array constructions pass `dtype=float` or `dtype=object` with a +`TODO: needed? DataTypes.NUMERIC?` comment. Decide whether explicit +dtype is needed and align with `DataTypes`. + +**TODOs:** + +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L415) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L423) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L431) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L456) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L466) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L474) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L543) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L556) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L624) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L637) +- [total_pd.py](src/easydiffraction/datablocks/experiment/categories/data/total_pd.py#L370) +- [total_pd.py](src/easydiffraction/datablocks/experiment/categories/data/total_pd.py#L378) + +**Depends on:** nothing. + +--- + +## 27. 🟢 Handle Zero Uncertainty in Bragg PD Data + +**Type:** Correctness + +A temporary workaround exists for zero uncertainties in measured data. + +**TODOs:** + +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L442) + +**Depends on:** nothing. + +--- + +## 28. 🟢 Clarify Bragg PD Data Collection Description + +**Type:** Cleanup + +`PdCwlDataCollection` has a commented-out `_description` and a +`TODO: ???` marker. + +**TODOs:** + +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L482) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L304) + +**Depends on:** nothing. + +--- + +## 29. 🟡 Standardise CIF ID Validator Pattern Across Categories + +**Type:** Consistency + +Multiple category item classes use the same regex `r'^[A-Za-z0-9_]*$'` +for their id/label validators with an identical TODO about CIF label vs. +internal label conversion. + +**TODOs:** + +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L43) +- [bragg_sc.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_sc.py#L39) +- [chebyshev.py](src/easydiffraction/datablocks/experiment/categories/background/chebyshev.py#L52) +- [line_segment.py](src/easydiffraction/datablocks/experiment/categories/background/line_segment.py#L45) +- [default.py](src/easydiffraction/datablocks/experiment/categories/excluded_regions/default.py#L39) +- [default.py](src/easydiffraction/datablocks/structure/categories/atom_sites/default.py#L45) + +**Depends on:** nothing. + +--- + +## 30. 🟢 Make `refinement_status` Default an Enum + +**Type:** Design + +`bragg_pd.py` uses `default='incl'` as a raw string with a TODO to make +it an Enum. The `_pd_data.refinement_status` CIF name should also be +renamed to `calc_status`. + +**TODOs:** + +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L113) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L118) + +**Depends on:** nothing. + +--- + +## 31. 🟢 Rename PD Data Point Mixins + +**Type:** Naming + +Mixin classes `PdDataPointBaseMixin` and `PdCwlDataPointMixin` have TODO +markers suggesting a rename to `BasePdDataPointMixin` and +`CwlPdDataPointMixin` for consistency. + +**TODOs:** + +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L268) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L269) +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/categories/data/bragg_pd.py#L271) + +**Depends on:** nothing. + +--- + +## 32. 🟡 Move Common Methods to `DatablockCollection` Base Class + +**Type:** Maintainability + +Both `Experiments` and `Structures` collections duplicate methods +(`from_cif_str`, `from_cif_file`, `show`, `show_as_cif`, etc.) that +could live in the base `DatablockCollection`. + +**TODOs:** + +- [collection.py](src/easydiffraction/datablocks/experiment/collection.py#L29) + — `Make abstract in DatablockCollection?` +- [collection.py](src/easydiffraction/datablocks/experiment/collection.py#L65) + — `Move to DatablockCollection?` +- [collection.py](src/easydiffraction/datablocks/experiment/collection.py#L82) +- [collection.py](src/easydiffraction/datablocks/experiment/collection.py#L145) +- [collection.py](src/easydiffraction/datablocks/experiment/collection.py#L151) +- [collection.py](src/easydiffraction/datablocks/structure/collection.py#L30) +- [collection.py](src/easydiffraction/datablocks/structure/collection.py#L48) +- [collection.py](src/easydiffraction/datablocks/structure/collection.py#L65) +- [collection.py](src/easydiffraction/datablocks/structure/collection.py#L82) +- [collection.py](src/easydiffraction/datablocks/structure/collection.py#L88) + +**Depends on:** nothing. + +--- + +## 33. 🟡 Make `DatablockItem._update_categories` Abstract + +**Type:** Design + +`DatablockItem._update_categories` has a TODO to make it abstract and +implement it in subclasses for structures (symmetry + constraints) and +experiments (calculation updates). Currently it is a concrete no-op. + +**TODOs:** + +- [datablock.py](src/easydiffraction/core/datablock.py#L39) + +**Depends on:** related to issue 11. + +--- + +## 34. 🟢 Auto-Extract `PeakProfileTypeEnum` from Peak Classes + +**Type:** Design + +Three related TODOs in `enums.py` ask whether `PeakProfileTypeEnum` +values can be auto-extracted from the actual peak profile classes in +`peak/cwl.py`, `tof.py`, `total.py` instead of being hardcoded, and +whether the same pattern can be reused for other enums. + +**TODOs:** + +- [enums.py](src/easydiffraction/datablocks/experiment/item/enums.py#L153) +- [enums.py](src/easydiffraction/datablocks/experiment/item/enums.py#L157) +- [enums.py](src/easydiffraction/datablocks/experiment/item/enums.py#L158) + +**Depends on:** related to issue 9. + +--- + +## 35. 🟢 Rename `BeamModeEnum` Members to CWL/TOF + +**Type:** Naming + +`BeamModeEnum.CONSTANT_WAVELENGTH` and `TIME_OF_FLIGHT` have a TODO to +be renamed to `CWL` and `TOF`. + +**TODOs:** + +- [enums.py](src/easydiffraction/datablocks/experiment/item/enums.py#L113) + +**Depends on:** nothing. + +--- + +## 36. 🟢 Consider a Common `EnumBase` with `default()` / `description()` + +**Type:** Design + +`BackgroundTypeEnum` and other enums repeat the same `default()` / +`description()` method pattern. A shared `EnumBase` would reduce +boilerplate. + +**TODOs:** + +- [enums.py](src/easydiffraction/datablocks/experiment/categories/background/enums.py#L10) + +**Depends on:** related to issue 9. + +--- + +## 37. 🟢 Rename Experiment `.type` Property + +**Type:** Naming + +`ExperimentBase.type` returns experimental metadata but the name shadows +the built-in `type`. A TODO suggests finding a better name. + +**TODOs:** + +- [base.py](src/easydiffraction/datablocks/experiment/item/base.py#L75) + +**Depends on:** nothing. + +--- + +## 38. 🟡 Fix `@typechecked` / gemmi Interaction in Factories + +**Type:** Bug + +Both `StructureFactory` and `ExperimentFactory` have `from_cif_str` +methods where `@typechecked` is commented out because it "fails to find +gemmi". They also share TODOs about adding minimal default configuration +for missing parameters and reading content from files. + +**TODOs:** + +- [factory.py](src/easydiffraction/datablocks/structure/item/factory.py#L41) +- [factory.py](src/easydiffraction/datablocks/structure/item/factory.py#L91) +- [factory.py](src/easydiffraction/datablocks/structure/item/factory.py#L115) +- [factory.py](src/easydiffraction/datablocks/experiment/item/factory.py#L59) + — `Add to core/factory.py?` +- [factory.py](src/easydiffraction/datablocks/experiment/item/factory.py#L108) +- [factory.py](src/easydiffraction/datablocks/experiment/item/factory.py#L177) +- [factory.py](src/easydiffraction/datablocks/experiment/item/factory.py#L201) + +**Depends on:** nothing. + +--- + +## 39. 🟢 Improve `_update_priority` Handling in Categories + +**Type:** Design + +`CategoryItem` and `CategoryCollection` both define +`_update_priority = 10` with a TODO to set different defaults and use +them during CIF serialisation. The duplicated `_update` no-op methods +are also marked. + +**TODOs:** + +- [category.py](src/easydiffraction/core/category.py#L21) +- [category.py](src/easydiffraction/core/category.py#L23) +- [category.py](src/easydiffraction/core/category.py#L32) +- [category.py](src/easydiffraction/core/category.py#L174) +- [category.py](src/easydiffraction/core/category.py#L199) + +**Depends on:** related to issues 10, 11. + +--- + +## 40. 🟢 Implement Resetting `.constrained` to `False` + +**Type:** Feature + +`ConstraintsHandler` has a TODO to implement changing the `.constrained` +attribute back to `False` when constraints are removed. + +**TODOs:** + +- [singleton.py](src/easydiffraction/core/singleton.py#L37) + +**Depends on:** nothing. + +--- + +## 41. 🟢 Check Whether `_mark_dirty` in `_set_value` is Actually Used + +**Type:** Cleanup + +`GenericDescriptorBase._set_value` marks the parent datablock dirty with +a TODO questioning whether this path is exercised. + +**TODOs:** + +- [variable.py](src/easydiffraction/core/variable.py#L154) + +**Depends on:** nothing. + +--- + +## 42. 🟢 MkDocs Doesn't Unpack Types in Validation Module + +**Type:** Docs + +A TODO in `validation.py` notes that MkDocs doesn't unpack types +properly. + +**TODOs:** + +- [validation.py](src/easydiffraction/core/validation.py#L25) + +**Depends on:** nothing. + +--- + +## 43. 🟢 Fix Summary Display Inconsistencies + +**Type:** UX + +The summary module has TODOs about fixing description wrapping and +inconsistent header capitalisation. + +**TODOs:** + +- [summary.py](src/easydiffraction/summary/summary.py#L52) +- [summary.py](src/easydiffraction/summary/summary.py#L164) + +**Depends on:** nothing. + +--- + +## 44. 🟢 Merge Parameter Record Construction in Analysis + +**Type:** Cleanup + +`Analysis._params_to_dataframe` has TODOs to merge record construction +for `StringDescriptor`/`NumericDescriptor`/`Parameter` and to use `repr` +formatting for `StringDescriptor` values. + +**TODOs:** + +- [analysis.py](src/easydiffraction/analysis/analysis.py#L461) +- [analysis.py](src/easydiffraction/analysis/analysis.py#L462) + +**Depends on:** nothing. + +--- + +## 45. 🟢 Decide Default for Alias/Constraint Descriptors + +**Type:** Design + +`Aliases` and `Constraints` categories use `default='_'` with a +`TODO, Maybe None?` marker. + +**TODOs:** + +- [default.py](src/easydiffraction/analysis/categories/aliases/default.py#L40) +- [default.py](src/easydiffraction/analysis/categories/constraints/default.py#L33) + +**Depends on:** nothing. + +--- + +## 46. 🟢 Rename `JointFitExperiments` ID and Improve Descriptions + +**Type:** Naming + +`JointFitExperiments` uses `name='id'` with a TODO suggesting a better +name, and two description fields are incomplete. + +**TODOs:** + +- [default.py](src/easydiffraction/analysis/categories/joint_fit_experiments/default.py#L33) +- [default.py](src/easydiffraction/analysis/categories/joint_fit_experiments/default.py#L34) +- [default.py](src/easydiffraction/analysis/categories/joint_fit_experiments/default.py#L43) + +**Depends on:** nothing. + +--- + +## 47. 🟢 Improve Error Handling in Crystallography Utilities + +**Type:** Diagnostics + +`crystallography.py` logs errors with a TODO asking whether these should +raise `ValueError` or provide better diagnostics. + +**TODOs:** + +- [crystallography.py](src/easydiffraction/crystallography/crystallography.py#L39) +- [crystallography.py](src/easydiffraction/crystallography/crystallography.py#L45) +- [crystallography.py](src/easydiffraction/crystallography/crystallography.py#L84) + +**Depends on:** nothing. + +--- + +## 48. 🟢 Fix CrysPy TOF Instrument Default + +**Type:** Bug workaround + +`TofInstrument.calib_d_to_tof_quad` defaults to `-0.00001` because +CrysPy does not accept `0`. + +**TODOs:** + +- [tof.py](src/easydiffraction/datablocks/experiment/categories/instrument/tof.py#L95) + +**Depends on:** upstream CrysPy fix. + +--- + +## 49. 🟢 Automate Space Group CIF Name Variants + +**Type:** Maintainability + +`SpaceGroup.name_h_m` lists multiple CIF tag variants (with `.` and +`_`). A TODO asks to keep only the dotted version and automate variant +generation. + +**TODOs:** + +- [default.py](src/easydiffraction/datablocks/structure/categories/space_group/default.py#L52) + +**Depends on:** nothing. + +--- + +## 50. 🟢 Clarify `Cell._update` Usage of `called_by_minimizer` + +**Type:** Cleanup + +`Cell._update` deletes `called_by_minimizer` with a `TODO: ???`. + +**TODOs:** + +- [default.py](src/easydiffraction/datablocks/structure/categories/cell/default.py#L146) + +**Depends on:** related to issue 11. + +--- + +## 51. 🟢 Access Space Group from `AtomSites` for Wyckoff Letters + +**Type:** Design + +`AtomSite` needs the current space group to determine allowed Wyckoff +letters but currently returns a hardcoded list. Also, a missing Wyckoff +letter case needs a decision. + +**TODOs:** + +- [default.py](src/easydiffraction/datablocks/structure/categories/atom_sites/default.py#L163) +- [default.py](src/easydiffraction/datablocks/structure/categories/atom_sites/default.py#L179) +- [default.py](src/easydiffraction/datablocks/structure/categories/atom_sites/default.py#L353) + +**Depends on:** nothing. + +--- + +## 52. 🟢 Rename Line-Segment Background `y` to `intensity` + +**Type:** Naming + +`LineSegmentBackgroundPoint.y` has TODOs to rename to `intensity`. + +**TODOs:** + +- [line_segment.py](src/easydiffraction/datablocks/experiment/categories/background/line_segment.py#L67) +- [line_segment.py](src/easydiffraction/datablocks/experiment/categories/background/line_segment.py#L72) + +**Depends on:** nothing. + +--- + +## 53. 🟢 Move `show()` to `CategoryCollection` Base Class + +**Type:** Maintainability + +`ExcludedRegions.show()` and `BackgroundBase.show()` duplicate table- +rendering logic. The TODO suggests moving it to the base class. + +**TODOs:** + +- [default.py](src/easydiffraction/datablocks/experiment/categories/excluded_regions/default.py#L166) +- [base.py](src/easydiffraction/datablocks/experiment/categories/background/base.py#L19) + +**Depends on:** nothing. + +--- + +## 54. 🟢 Add `point_id` to Excluded Regions + +**Type:** Completeness + +`ExcludedRegion` has a TODO to add `point_id` similar to background +categories. + +**TODOs:** + +- [default.py](src/easydiffraction/datablocks/experiment/categories/excluded_regions/default.py#L33) + +**Depends on:** nothing. + +--- + +## 55. 🟢 Fix Jupyter Scroll Disabling for MkDocs + +**Type:** Docs / UX + +`display/__init__.py` has disabled `JupyterScrollManager` because it +breaks MkDocs builds. + +**TODOs:** + +- [**init**.py](src/easydiffraction/display/__init__.py#L15) + +**Depends on:** nothing. + +--- + +## 56. 🟢 Make ASCII Plot Width Configurable + +**Type:** UX + +`ascii.py` hardcodes `width = 60` with a TODO to make it configurable. + +**TODOs:** + +- [ascii.py](src/easydiffraction/display/plotters/ascii.py#L144) +- [ascii.py](src/easydiffraction/display/plotters/ascii.py#L98) + +**Depends on:** nothing. + +--- + +## 57. 🟢 Clean Up CIF Deserialisation Helpers + +**Type:** Maintainability + +`serialize.py` has several TODOs: verify methods after the +`format_param_value` section, extract a helper for quoted-string +stripping, find a better way to set `_item_type` on +`CategoryCollection`, rename it to `_item_cls`, and remove duplicated +`param_from_cif` logic. + +**TODOs:** + +- [serialize.py](src/easydiffraction/io/cif/serialize.py#L454) +- [serialize.py](src/easydiffraction/io/cif/serialize.py#L562) +- [serialize.py](src/easydiffraction/io/cif/serialize.py#L617) +- [serialize.py](src/easydiffraction/io/cif/serialize.py#L619) +- [serialize.py](src/easydiffraction/io/cif/serialize.py#L656) + +**Depends on:** nothing. + +--- + +## 58. 🟢 Move `as_cif` / `show_as_cif` from `ProjectInfo` to `io.cif.serialize` + +**Type:** Maintainability + +`ProjectInfo` methods `as_cif` and `show_as_cif` have TODOs suggesting +they belong in the serialisation module. + +**TODOs:** + +- [project_info.py](src/easydiffraction/project/project_info.py#L123) +- [project_info.py](src/easydiffraction/project/project_info.py#L128) + +**Depends on:** nothing. + +--- + +## 59. 🟢 Add CIF Name Validation or Normalisation in Parse + +**Type:** Robustness + +`io/cif/parse.py` has a TODO about adding a validator or normalisation +step. + +**TODOs:** + +- [parse.py](src/easydiffraction/io/cif/parse.py#L29) + +**Depends on:** nothing. + +--- + +## 60. 🟢 Unify `mkdir` Usage Across the Codebase + +**Type:** Cleanup + +`io/ascii.py` has a TODO to unify directory creation with other uses. + +**TODOs:** + +- [ascii.py](src/easydiffraction/io/ascii.py#L118) + +**Depends on:** nothing. + +--- + +## 61. 🟢 Clarify Logger Default Reaction Mode + +**Type:** Design + +`Logger._reaction` defaults to `Reaction.RAISE` with a +`TODO: not default?` marker. + +**TODOs:** + +- [logging.py](src/easydiffraction/utils/logging.py#L430) + +**Depends on:** nothing. + +--- + +## 62. 🟢 Complete Migration from `render_table` to `TableRenderer` + +**Type:** Cleanup + +`utils.py` has a temporary `render_table` utility that should be +replaced with `TableRenderer`. + +**TODOs:** + +- [utils.py](src/easydiffraction/utils/utils.py#L510) + +**Depends on:** nothing. + +--- + +## 63. 🟢 Fix Calculator `calculate_pattern` Signature Type + +**Type:** Design + +`CalculatorBase.calculate_pattern` takes `structure: Structures` but the +TODO asks whether it should be `Structure` (singular). + +**TODOs:** + +- [base.py](src/easydiffraction/analysis/calculators/base.py#L40) + +**Depends on:** nothing. + +--- + +## 64. 🟢 Check Whether `_not_used_if_loading_from_cif` Code is Needed + +**Type:** Cleanup + +`BraggPdExperiment` has a block marked +`TODO: Not used if loading from cif file?`. + +**TODOs:** + +- [bragg_pd.py](src/easydiffraction/datablocks/experiment/item/bragg_pd.py#L112) + +**Depends on:** nothing. + +--- + +## 65. 🟡 Replace All Bare `print()` Calls with Logging + +**Type:** Code quality + +~22 bare `print()` calls exist in `src/` (not `console.print()`, not +commented out). All output should go through `log` or `console` so that +verbosity is controllable. Key offenders: `fitting.py`, `sequential.py`, +`ascii.py`, `base.py` (experiment), calculator modules, `singleton.py`. + +**Depends on:** nothing. + +--- + +## 66. 🟡 Decide Error-Handling Strategy: `log.error` vs `raise` + +**Type:** Design + +The codebase mixes `log.error(msg)` (which may raise depending on +`Reaction` mode) and direct `raise ValueError(...)`. A consistent +strategy is needed: when to use `log.error` (user-facing, recoverable) +vs native exceptions (programmer errors, unrecoverable). This also +relates to the `Reaction` mode setting (issue 61). + +**Depends on:** issue 61. + +--- + +## 67. 🟡 Custom Validation for Parameter/Descriptor and Category Types + +**Type:** Design + +Parameters and Descriptors use `RangeValidator`, `RegexValidator`, +`MembershipValidator` for values but rely on `@typechecked` (only in +some places) for type checking. Category switchable types use different +validation paths. Decide whether to: + +- Use custom validators for both types and values on Parameters. +- Use custom validators for category type setters. +- Standardise the approach across the codebase. + +**Depends on:** issue 38 (`@typechecked` / gemmi interaction). + +--- + +## 68. 🟢 Decide Whether to Apply `@typechecked` to All Public Methods + +**Type:** Design + +`@typechecked` is currently applied only in ~24 places (factories, +collections). Decide whether it should be applied systematically to all +public method signatures, or whether custom validation (issue 67) is +preferred. + +**Depends on:** issue 67. + +--- + +## 69. 🟢 Shorter Public API Names via `__init__.py` Re-Exports + +**Type:** API ergonomics + +Classes are imported in `__init__.py` files but users still need deep +paths to reach them. Consider whether top-level re-exports (e.g. +`from easydiffraction import Project, Structure, Experiment`) should +provide shorter access, and document the policy. + +**Depends on:** nothing. + +--- + +## 70. 🟡 Standardise Class Member Ordering and Visual Section Headers + +**Type:** Code style + +Agree on and enforce a consistent ordering within every class: + +1. Class-level attributes / metadata +2. `__init__` +3. Private helper methods +4. Public properties (getters/setters) +5. Public methods + +Each group should have a comment header (e.g. +`# --- Public properties ---`) for visual separation. Some classes +already use this pattern; apply it uniformly. + +**Depends on:** nothing. + +--- + +## 71. 🟢 Create `_update_priority` Reference Table for Categories + +**Type:** Documentation + +Create and maintain a table listing all categories that implement +`_update()`, sorted by their `_update_priority`. This makes the update +order explicit and helps catch priority conflicts. + +**Depends on:** related to issues 11, 39. + +--- + +## 72. 🟡 Warn on All Switchable-Category Type Changes + +**Type:** UX / Consistency + +Switching `background_type` already warns: "Switching background type +discards 1 existing background point(s)." The same warning pattern +should apply to all other switchable types (`peak_profile_type`, +`data_type`, etc.) so users know their values will be lost. + +**Depends on:** nothing. + +--- + +## 73. 🟢 Unify Setter Parameter Naming Convention + +**Type:** Code style + +Some setters use `new`, others use `value`, others use the attribute +name. For example: + +```python +@id.setter +def id(self, new): + self._id.value = new +``` + +Agree on a single convention (e.g. always `value`) and apply +consistently. + +**Depends on:** nothing. + +--- + +## 74. 🟡 Sync Property Type Hints with Private Attributes + Custom Lint + +**Type:** Tooling / Correctness + +Public property getters return `Parameter` / `StringDescriptor` etc., +and setters accept `float` / `str` etc. These annotations must stay in +sync with the private `_attr` type. Currently there is no automated +check. Options: + +- A custom script (like `param_consistency.py`) to verify sync. +- A ruff plugin or post-ruff check step. +- Also covers: enforcing `Base` suffix (not prefix), checking missing + docstrings (issue 81), and other project-specific conventions. + +**Depends on:** nothing. + +--- + +## 75. 🟢 Add `show_supported_calculators()` on Analysis or Project + +**Type:** API completeness + +`show_supported_calculator_types()` exists per-experiment, but there is +no project/analysis-level method to list all available calculator +engines. Users exploring the API have no single entry point to see what +calculators are installed. + +**Depends on:** nothing. + +--- + +## 76. 🟡 Consistent `_type` Suffix in Switchable-Category API Names + +**Type:** Naming / Consistency + +The switchable-category naming convention prescribes `_type` +(getter/setter) and `show_supported__types()`. But some names +deviate: e.g. `show_supported_minimizers()` instead of +`show_supported_minimizer_types()`, and `current_minimizer` instead of +`minimizer_type`. Audit and align all switchable-category APIs. + +**Depends on:** nothing. + +--- + +## 77. 🟡 Add `help()` to `Project` and Enrich Existing `help()` Methods + +**Type:** API discoverability + +`help()` exists on `CategoryItem`, `CollectionBase`, `DatablockItem`, +and `Analysis`, but **not on `Project`**. The user's primary entry point +lacks discoverability. Additionally, each `help()` level should guide +the user to the next level: + +1. `project.help()` → attributes: info, experiments, structures, + analysis, summary. +2. `project.experiments.help()` → list experiments and how to select. +3. `project.experiments['name'].help()` → list categories. +4. `experiment.peak.help()` → list public attributes. +5. `experiment.background.help()` → list items + array accessors. +6. `experiment.background['id'].help()` → list attributes. + +**Depends on:** nothing. + +--- + +## 79. 🟢 Verify Completeness of Analysis CIF Serialisation + +**Type:** Correctness + +`analysis_to_cif()` and `analysis_from_cif()` exist, but audit whether +**all** analysis state is persisted: aliases, constraints, fit mode, +joint-fit weights, minimiser type, calculator assignments. Any missing +fields means a loaded project silently differs from the saved one. + +**Depends on:** related to issue 16. + +--- + +## 80. 🟢 Resolve `Any` vs `object` Type Annotation Policy + +**Type:** Code style + +Both `Any` and `object` are used as generic parameter types. Current +pattern: `Any` inside containers (`dict[str, Any]`), `object` for +standalone params (often to avoid circular imports). Decide on a policy: + +- Use protocol types / `TYPE_CHECKING` imports instead of `object`. +- Reserve `Any` for genuinely unknown types. +- Document when each is appropriate. + +**Depends on:** nothing. + +--- + +## 81. 🟡 Enforce Docstrings on All Public Methods + +**Type:** Code quality + +Some public methods (e.g. `plot_meas_vs_calc`, others) lack docstrings. +Decide: + +- All public methods **must** have numpy-style docstrings. +- Private helpers: minimal one-liner docstring or none? Choose a policy. +- Enable a ruff rule (e.g. `D103`, `D102`) or add a custom check to + enforce. + +**Depends on:** nothing. + +--- + +## 82. 🟢 Document `param-docstring-fix` and `notebook-prepare` Workflow + +**Type:** Documentation + +Two manual workflow steps are required between releases/changes: + +1. `pixi run param-docstring-fix` — sync Parameter docstrings. +2. `pixi run notebook-prepare` — regenerate tutorial notebooks from + scripts. + +Document these in `CONTRIBUTING.md` or the architecture doc so they are +not forgotten. + +**Depends on:** nothing. + +--- + +## 83. 🟢 Remove Redundant Parameter Listing from Parameter Itself + +**Type:** Cleanup + +Parameters currently carry some form of self-listing metadata that is +redundant with the category/collection level. Remove it to keep the +single-responsibility principle. + +**Depends on:** nothing. + +--- + +## 84. 🟡 Serialise `None` as `.` in CIF Output + +**Type:** Correctness + +CIF output currently writes `None` as literal text for some fields (e.g. +`_diffrn.ambient_pressure None`). CIF convention uses `.` for +inapplicable values and `?` for unknown. The CIF writer should map +`None` → `.` (or `?` depending on semantics), and the reader should map +`.` → `None`. + +**Depends on:** nothing. + +--- + +## 85. 🟡 Retain Per-Experiment Fitted Parameters for Plotting + +**Type:** Correctness / UX + +In `single` fit mode, only the last experiment's fit results are +retained because structure parameters are overwritten on each iteration. +This means earlier experiments cannot be plotted correctly after +fitting. + +**Fix:** after fitting each experiment, store a snapshot of its fitted +parameters (both structure and experiment) so that any experiment can be +re-plotted or inspected later. Also clarify: does `fit_results` need to +keep the last mutable parameter set after adding a snapshot? + +**Depends on:** nothing (issue 78 resolved). + +--- + +## 86. 🟢 Auto-Resolve `plot_param` X-Axis Descriptor and Add Units + +**Type:** UX + +`plot_param_series` currently requires the user to manually specify the +x-axis parameter (e.g. `x_axis='temperature'` → look up +`diffrn.ambient_temperature`). This should be auto-resolved from the +parameter name. Additionally, axis labels should include units (e.g. +"Temperature (K)"). + +**Depends on:** nothing. + +--- + +## 87. 🟢 Redesign Tutorial Grouping and Categorisation + +**Type:** Documentation / UX + +The current tutorial index uses a flat `"level": "advanced"` tag. A +richer categorisation system is needed: + +- Group by topic (matching the docs structure). +- Multiple difficulty levels. +- Multiple Python-knowledge levels. + +**Depends on:** nothing. + +--- + +## 88. 🟢 Fix Dataset 26 Description (47 Files, Not 57) + +**Type:** Data + +Dataset 26 description says "57 files" but should say "47 files": +`"Co2SiO4, D20 (ILL), 57 files, T from ~50K to ~500K"` → +`"Co2SiO4, D20 (ILL), 47 files, T from ~50K to ~500K"`. + +**Depends on:** nothing. + +--- + +## 89. 🟡 Parallel Independent Fits for Single/Independent Fit Mode + +**Type:** Performance + +In `single` (independent) fit mode, each experiment has its own +structure parameters and is completely independent. These fits could run +in parallel threads. Sequential mode, by contrast, must remain single- +threaded because each step's output is the next step's input. + +**Depends on:** nothing (issue 78 resolved). + +--- + +## 90. 🟢 Show Experiment Number/Total During Sequential Fitting + +**Type:** UX + +Currently prints: `Using experiment 🔬 'd20_30' for 'single' fitting` + +Should print: +`Using experiment 🔬 'd20_30' (No. 30 of 47) for 'single' fitting` + +**Depends on:** nothing. + +--- + +## 91. 🟢 Disable TODO Comment Checks in CodeFactor PRs + +**Type:** CI / Tooling + +CodeFactor flags TODO comments as unresolved issues (rule C100) in PRs. +Since TODOs are tracked in `issues_open.md`, the CodeFactor check adds +noise. Disable the C100 rule or configure CodeFactor to ignore TODO +comments. + +**Depends on:** nothing. + +--- + +## 92. 🟢 Make `save()` Respect Verbosity Settings + +**Type:** UX + +`Project.save()` unconditionally prints progress via `console.print()`. +It should respect the logger's verbosity mode so that silent/quiet +operation is possible (e.g. in automated pipelines or tests). + +**Depends on:** nothing. + +--- + ## Summary -| # | Issue | Severity | Type | -| --- | ---------------------------------------- | -------- | ----------------------- | -| 2 | Restore minimiser variants | 🟡 Med | Feature loss | -| 3 | Rebuild joint-fit weights | 🟡 Med | Fragility | -| 5 | `Analysis` as `DatablockItem` | 🟡 Med | Consistency | -| 6 | Restrict `data_type` switching | 🔴 High | Correctness/Data safety | -| 8 | Explicit `create()` signatures | 🟡 Med | API safety | -| 9 | Future enum extensions | 🟢 Low | Design | -| 10 | Unify update orchestration | 🟢 Low | Maintainability | -| 11 | Document `_update` contract | 🟢 Low | Maintainability | -| 13 | Suppress redundant dirty-flag sets | 🟢 Low | Performance | -| 14 | Finer-grained change tracking | 🟢 Low | Performance | -| 15 | Validate joint-fit weights | 🟡 Med | Correctness | -| 16 | Persist per-experiment `calculator_type` | 🟡 Med | Completeness | +| # | Issue | Severity | Type | +| --- | ------------------------------------------------ | -------- | ---------------- | +| 3 | Rebuild joint-fit weights | 🟡 Med | Fragility | +| 5 | `Analysis` as `DatablockItem` | 🟡 Med | Consistency | +| 8 | Explicit `create()` signatures | 🟡 Med | API safety | +| 9 | Future enum extensions | 🟢 Low | Design | +| 10 | Unify update orchestration | 🟢 Low | Maintainability | +| 11 | Document `_update` contract | 🟢 Low | Maintainability | +| 13 | Suppress redundant dirty-flag sets | 🟢 Low | Performance | +| 14 | Finer-grained change tracking | 🟢 Low | Performance | +| 15 | Validate joint-fit weights | 🟡 Med | Correctness | +| 16 | Persist per-experiment `calculator_type` | 🟡 Med | Completeness | +| 17 | Use PDF-specific CIF names | 🟢 Low | Naming | +| 18 | Move CIF v2→v1 conversion out of calculator | 🟢 Low | Maintainability | +| 19 | Debug-mode logging for calculator imports | 🟢 Low | Diagnostics | +| 20 | Redirect/suppress CrysPy stderr | 🟢 Low | UX | +| 21 | Clarify CrysPy TOF background CIF tags | 🟡 Med | Correctness | +| 22 | Check SC instrument mapping in CrysPy | 🟢 Low | Correctness | +| 23 | Investigate PyCrysFML pattern length discrepancy | 🟢 Low | Correctness | +| 24 | Process defaults on experiment creation | 🟢 Low | Design | +| 25 | Refactor data `_update` methods | 🟡 Med | Maintainability | +| 26 | Clarify `dtype` usage in data arrays | 🟢 Low | Cleanup | +| 27 | Handle zero uncertainty in Bragg PD | 🟢 Low | Correctness | +| 28 | Clarify Bragg PD data collection description | 🟢 Low | Cleanup | +| 29 | Standardise CIF ID validator pattern | 🟡 Med | Consistency | +| 30 | Make `refinement_status` default an Enum | 🟢 Low | Design | +| 31 | Rename PD data point mixins | 🟢 Low | Naming | +| 32 | Move common methods to `DatablockCollection` | 🟡 Med | Maintainability | +| 33 | Make `_update_categories` abstract | 🟡 Med | Design | +| 34 | Auto-extract `PeakProfileTypeEnum` | 🟢 Low | Design | +| 35 | Rename `BeamModeEnum` members to CWL/TOF | 🟢 Low | Naming | +| 36 | Common `EnumBase` class | 🟢 Low | Design | +| 37 | Rename experiment `.type` property | 🟢 Low | Naming | +| 38 | Fix `@typechecked`/gemmi in factories | 🟡 Med | Bug | +| 39 | Improve `_update_priority` handling | 🟢 Low | Design | +| 40 | Implement resetting `.constrained` to `False` | 🟢 Low | Feature | +| 41 | Check `_mark_dirty` in `_set_value` | 🟢 Low | Cleanup | +| 42 | MkDocs type unpacking in validation | 🟢 Low | Docs | +| 43 | Fix summary display inconsistencies | 🟢 Low | UX | +| 44 | Merge parameter record construction | 🟢 Low | Cleanup | +| 45 | Decide alias/constraint descriptor default | 🟢 Low | Design | +| 46 | Rename `JointFitExperiments` id + descriptions | 🟢 Low | Naming | +| 47 | Improve error handling in crystallography | 🟢 Low | Diagnostics | +| 48 | Fix CrysPy TOF instrument default | 🟢 Low | Bug workaround | +| 49 | Automate space group CIF name variants | 🟢 Low | Maintainability | +| 50 | Clarify `Cell._update` minimizer param | 🟢 Low | Cleanup | +| 51 | Access space group for Wyckoff letters | 🟢 Low | Design | +| 52 | Rename line-segment `y` to `intensity` | 🟢 Low | Naming | +| 53 | Move `show()` to `CategoryCollection` | 🟢 Low | Maintainability | +| 54 | Add `point_id` to excluded regions | 🟢 Low | Completeness | +| 55 | Fix Jupyter scroll disabling for MkDocs | 🟢 Low | Docs / UX | +| 56 | Make ASCII plot width configurable | 🟢 Low | UX | +| 57 | Clean up CIF deserialisation helpers | 🟢 Low | Maintainability | +| 58 | Move `ProjectInfo` CIF methods to `serialize` | 🟢 Low | Maintainability | +| 59 | Add CIF name validation in parse | 🟢 Low | Robustness | +| 60 | Unify `mkdir` usage | 🟢 Low | Cleanup | +| 61 | Clarify logger default reaction mode | 🟢 Low | Design | +| 62 | Complete `render_table` → `TableRenderer` | 🟢 Low | Cleanup | +| 63 | Fix calculator `calculate_pattern` signature | 🟢 Low | Design | +| 64 | Check unused-if-loading-from-CIF code | 🟢 Low | Cleanup | +| 65 | Replace all bare `print()` with logging | 🟡 Med | Code quality | +| 66 | Error-handling strategy: `log.error` vs `raise` | 🟡 Med | Design | +| 67 | Custom validation for params and category types | 🟡 Med | Design | +| 68 | `@typechecked` on all public methods? | 🟢 Low | Design | +| 69 | Shorter public API names via `__init__` | 🟢 Low | API ergonomics | +| 70 | Standardise class member ordering + headers | 🟡 Med | Code style | +| 71 | `_update_priority` reference table | 🟢 Low | Documentation | +| 72 | Warn on all switchable-category type changes | 🟡 Med | UX | +| 73 | Unify setter parameter naming | 🟢 Low | Code style | +| 74 | Sync property type hints + custom lint rules | 🟡 Med | Tooling | +| 75 | `show_supported_calculators()` on Analysis | 🟢 Low | API completeness | +| 76 | Consistent `_type` suffix in switchable APIs | 🟡 Med | Naming | +| 77 | Add `help()` to Project + enrich existing | 🟡 Med | Discoverability | +| 79 | Verify analysis CIF serialisation completeness | 🟢 Low | Correctness | +| 80 | Resolve `Any` vs `object` annotation policy | 🟢 Low | Code style | +| 81 | Enforce docstrings on all public methods | 🟡 Med | Code quality | +| 82 | Document `param-docstring-fix` workflow | 🟢 Low | Documentation | +| 83 | Remove redundant parameter listing | 🟢 Low | Cleanup | +| 84 | Serialise `None` as `.` in CIF output | 🟡 Med | Correctness | +| 85 | Retain per-experiment fitted params for plotting | 🟡 Med | Correctness | +| 86 | Auto-resolve `plot_param` x-axis + add units | 🟢 Low | UX | +| 87 | Redesign tutorial grouping/categorisation | 🟢 Low | Documentation | +| 88 | Fix Dataset 26 description (47 not 57) | 🟢 Low | Data | +| 89 | Parallel independent fits for single mode | 🟡 Med | Performance | +| 90 | Show experiment number during sequential fitting | 🟢 Low | UX | +| 91 | Disable TODO checks in CodeFactor PRs | 🟢 Low | CI / Tooling | +| 92 | Make `save()` respect verbosity | 🟢 Low | UX | diff --git a/docs/architecture/package-structure-full.md b/docs/architecture/package-structure-full.md index a95724f8d..c4bf5b5f1 100644 --- a/docs/architecture/package-structure-full.md +++ b/docs/architecture/package-structure-full.md @@ -58,12 +58,27 @@ │ │ ├── 📄 __init__.py │ │ ├── 📄 base.py │ │ │ └── 🏷️ class MinimizerBase +│ │ ├── 📄 bumps.py +│ │ │ ├── 🏷️ class _EasyDiffractionFitness +│ │ │ └── 🏷️ class BumpsMinimizer +│ │ ├── 📄 bumps_amoeba.py +│ │ │ └── 🏷️ class BumpsAmoebaMinimizer +│ │ ├── 📄 bumps_de.py +│ │ │ └── 🏷️ class BumpsDEMinimizer +│ │ ├── 📄 bumps_lm.py +│ │ │ └── 🏷️ class BumpsLmMinimizer │ │ ├── 📄 dfols.py │ │ │ └── 🏷️ class DfolsMinimizer +│ │ ├── 📄 enums.py +│ │ │ └── 🏷️ class MinimizerTypeEnum │ │ ├── 📄 factory.py │ │ │ └── 🏷️ class MinimizerFactory -│ │ └── 📄 lmfit.py -│ │ └── 🏷️ class LmfitMinimizer +│ │ ├── 📄 lmfit.py +│ │ │ └── 🏷️ class LmfitMinimizer +│ │ ├── 📄 lmfit_least_squares.py +│ │ │ └── 🏷️ class LmfitLeastSquaresMinimizer +│ │ └── 📄 lmfit_leastsq.py +│ │ └── 🏷️ class LmfitLeastsqMinimizer │ ├── 📄 __init__.py │ ├── 📄 analysis.py │ │ ├── 🏷️ class AnalysisDisplay @@ -119,6 +134,7 @@ │ ├── 📄 __init__.py │ ├── 📄 crystallography.py │ └── 📄 space_groups.py +│ └── 🏷️ class _RestrictedUnpickler ├── 📁 datablocks │ ├── 📁 experiment │ │ ├── 📁 categories @@ -177,10 +193,10 @@ │ │ │ │ └── 🏷️ class ExperimentTypeFactory │ │ │ ├── 📁 extinction │ │ │ │ ├── 📄 __init__.py -│ │ │ │ ├── 📄 factory.py -│ │ │ │ │ └── 🏷️ class ExtinctionFactory -│ │ │ │ └── 📄 becker_coppens.py -│ │ │ │ └── 🏷️ class BeckerCoppensExtinction +│ │ │ │ ├── 📄 becker_coppens.py +│ │ │ │ │ └── 🏷️ class BeckerCoppensExtinction +│ │ │ │ └── 📄 factory.py +│ │ │ │ └── 🏷️ class ExtinctionFactory │ │ │ ├── 📁 instrument │ │ │ │ ├── 📄 __init__.py │ │ │ │ ├── 📄 base.py @@ -252,7 +268,8 @@ │ │ │ │ ├── 🏷️ class RadiationProbeEnum │ │ │ │ ├── 🏷️ class BeamModeEnum │ │ │ │ ├── 🏷️ class CalculatorEnum -│ │ │ │ └── 🏷️ class PeakProfileTypeEnum +│ │ │ │ ├── 🏷️ class PeakProfileTypeEnum +│ │ │ │ └── 🏷️ class ExtinctionModelEnum │ │ │ ├── 📄 factory.py │ │ │ │ └── 🏷️ class ExperimentFactory │ │ │ └── 📄 total_pd.py diff --git a/docs/architecture/package-structure-short.md b/docs/architecture/package-structure-short.md index a26495b04..5c570ad9f 100644 --- a/docs/architecture/package-structure-short.md +++ b/docs/architecture/package-structure-short.md @@ -37,9 +37,16 @@ │ ├── 📁 minimizers │ │ ├── 📄 __init__.py │ │ ├── 📄 base.py +│ │ ├── 📄 bumps.py +│ │ ├── 📄 bumps_amoeba.py +│ │ ├── 📄 bumps_de.py +│ │ ├── 📄 bumps_lm.py │ │ ├── 📄 dfols.py +│ │ ├── 📄 enums.py │ │ ├── 📄 factory.py -│ │ └── 📄 lmfit.py +│ │ ├── 📄 lmfit.py +│ │ ├── 📄 lmfit_least_squares.py +│ │ └── 📄 lmfit_leastsq.py │ ├── 📄 __init__.py │ ├── 📄 analysis.py │ ├── 📄 fitting.py @@ -91,8 +98,8 @@ │ │ │ │ └── 📄 factory.py │ │ │ ├── 📁 extinction │ │ │ │ ├── 📄 __init__.py -│ │ │ │ ├── 📄 factory.py -│ │ │ │ └── 📄 becker_coppens.py +│ │ │ │ ├── 📄 becker_coppens.py +│ │ │ │ └── 📄 factory.py │ │ │ ├── 📁 instrument │ │ │ │ ├── 📄 __init__.py │ │ │ │ ├── 📄 base.py diff --git a/docs/docs/tutorials/data/lbco_project/summary.cif b/docs/docs/tutorials/data/lbco_project/summary.cif new file mode 100644 index 000000000..ccc4df039 --- /dev/null +++ b/docs/docs/tutorials/data/lbco_project/summary.cif @@ -0,0 +1 @@ +To be added... \ No newline at end of file diff --git a/docs/docs/tutorials/ed-17.py b/docs/docs/tutorials/ed-17.py index ce8b7ec01..5feb94e83 100644 --- a/docs/docs/tutorials/ed-17.py +++ b/docs/docs/tutorials/ed-17.py @@ -259,6 +259,12 @@ # %% project.analysis.constraints.create(expression='biso_Co2 = biso_Co1') +# %% [markdown] +# #### Set Minimizer + +# %% +project.analysis.current_minimizer = 'bumps (lm)' + # %% [markdown] # #### Run Single Fitting # diff --git a/docs/docs/tutorials/ed-3.py b/docs/docs/tutorials/ed-3.py index a8b84b7da..d6ad73598 100644 --- a/docs/docs/tutorials/ed-3.py +++ b/docs/docs/tutorials/ed-3.py @@ -76,14 +76,6 @@ # %% project.plotter.show_config() -# %% [markdown] -# Set plotting engine. - -# %% -# Keep the auto-selected engine. Alternatively, you can uncomment the -# line below to explicitly set the engine to the required one. -# project.plotter.engine = 'plotly' - # %% [markdown] # ## Step 2: Define Structure # diff --git a/docs/docs/tutorials/ed-5.py b/docs/docs/tutorials/ed-5.py index 58a339f0d..2938e50de 100644 --- a/docs/docs/tutorials/ed-5.py +++ b/docs/docs/tutorials/ed-5.py @@ -127,6 +127,7 @@ # #### Set Peak Profile # %% +expt.peak_profile_type = 'pseudo-voigt + empirical asymmetry' expt.peak.broad_gauss_u = 0.3 expt.peak.broad_gauss_v = -0.5 expt.peak.broad_gauss_w = 0.4 @@ -167,14 +168,6 @@ # %% project = Project() -# %% [markdown] -# #### Set Plotting Engine - -# %% -# Keep the auto-selected engine. Alternatively, you can uncomment the -# line below to explicitly set the engine to the required one. -# project.plotter.engine = 'plotly' - # %% [markdown] # #### Add Structure @@ -244,6 +237,8 @@ expt.peak.broad_gauss_w.free = True expt.peak.broad_lorentz_y.free = True +expt.peak.asym_empir_2.free = True + for point in expt.background: point.y.free = True @@ -266,9 +261,7 @@ # Set constraints. # %% -project.analysis.constraints.create( - expression='biso_Co2 = biso_Co1', -) +project.analysis.constraints.create(expression='biso_Co2 = biso_Co1') # %% [markdown] @@ -276,8 +269,13 @@ # %% project.analysis.fit() + +# %% project.analysis.display.fit_results() +# %% +project.plotter.plot_param_correlations() + # %% [markdown] # #### Plot Measured vs Calculated diff --git a/pixi.toml b/pixi.toml index f4073f9ef..6706f3c9f 100644 --- a/pixi.toml +++ b/pixi.toml @@ -136,6 +136,7 @@ py-lint-fix-unsafe = 'ruff check --fix --unsafe-fixes src/ tests/ docs/docs/tuto py-format-fix = 'ruff format src/ tests/ docs/docs/tutorials/' nonpy-format-fix = 'npx prettier --write --list-different --config=prettierrc.toml --ignore-unknown .' nonpy-format-fix-modified = 'python tools/nonpy_prettier_modified.py --write' +update-package-diagrams = 'python tools/generate_package_docs.py' success-message = 'echo "✅ All auto-formatting steps completed successfully!"' fix = { depends-on = [ @@ -145,6 +146,7 @@ fix = { depends-on = [ 'py-lint-fix', 'nonpy-format-fix', 'notebook-lint-fix', + 'update-package-diagrams', 'success-message', ] } diff --git a/src/easydiffraction/analysis/analysis.py b/src/easydiffraction/analysis/analysis.py index 8a641bc9f..9586860b8 100644 --- a/src/easydiffraction/analysis/analysis.py +++ b/src/easydiffraction/analysis/analysis.py @@ -400,7 +400,7 @@ def __init__(self, project: object) -> None: self._fit_mode_type: str = FitModeFactory.default_tag() self._fit_mode = FitModeFactory.create(self._fit_mode_type) self._joint_fit_experiments = JointFitExperiments() - self.fitter = Fitter('lmfit') + self.fitter = Fitter() self.fit_results = None self._parameter_snapshots: dict[str, dict[str, dict]] = {} self._display = AnalysisDisplay(self) @@ -435,87 +435,9 @@ def help(self) -> None: ) # ------------------------------------------------------------------ - # Aliases (switchable-category pattern) + # Parameter helpers # ------------------------------------------------------------------ - @property - def aliases_type(self) -> str: - """Tag of the active aliases collection type.""" - return self._aliases_type - - @aliases_type.setter - def aliases_type(self, new_type: str) -> None: - """ - Switch to a different aliases collection type. - - Parameters - ---------- - new_type : str - Aliases tag (e.g. ``'default'``). - """ - supported_tags = AliasesFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported aliases type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_aliases_types()'", - ) - return - self.aliases = AliasesFactory.create(new_type) - self._aliases_type = new_type - console.paragraph('Aliases type changed to') - console.print(new_type) - - def show_supported_aliases_types(self) -> None: # noqa: PLR6301 - """Print a table of supported aliases collection types.""" - AliasesFactory.show_supported() - - def show_current_aliases_type(self) -> None: - """Print the currently used aliases collection type.""" - console.paragraph('Current aliases type') - console.print(self._aliases_type) - - # ------------------------------------------------------------------ - # Constraints (switchable-category pattern) - # ------------------------------------------------------------------ - - @property - def constraints_type(self) -> str: - """Tag of the active constraints collection type.""" - return self._constraints_type - - @constraints_type.setter - def constraints_type(self, new_type: str) -> None: - """ - Switch to a different constraints collection type. - - Parameters - ---------- - new_type : str - Constraints tag (e.g. ``'default'``). - """ - supported_tags = ConstraintsFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported constraints type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_constraints_types()'", - ) - return - self.constraints = ConstraintsFactory.create(new_type) - self._constraints_type = new_type - console.paragraph('Constraints type changed to') - console.print(new_type) - - def show_supported_constraints_types(self) -> None: # noqa: PLR6301 - """Print a table of supported constraints collection types.""" - ConstraintsFactory.show_supported() - - def show_current_constraints_type(self) -> None: - """Print the currently used constraints collection type.""" - console.paragraph('Current constraints type') - console.print(self._constraints_type) - @staticmethod def _get_params_as_dataframe( params: list[NumericDescriptor | Parameter], @@ -595,7 +517,7 @@ def current_minimizer(self, selection: str) -> None: console.print(self.current_minimizer) # ------------------------------------------------------------------ - # Fit mode (switchable-category pattern) + # Fit mode (single type, with show methods) # ------------------------------------------------------------------ @property @@ -603,42 +525,25 @@ def fit_mode(self) -> object: """Fit-mode category item holding the active strategy.""" return self._fit_mode - @property - def fit_mode_type(self) -> str: - """Tag of the active fit-mode category type.""" - return self._fit_mode_type - - @fit_mode_type.setter - def fit_mode_type(self, new_type: str) -> None: - """ - Switch to a different fit-mode category type. - - Parameters - ---------- - new_type : str - Fit-mode tag (e.g. ``'default'``). - """ - supported_tags = FitModeFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported fit-mode type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_fit_mode_types()'", - ) - return - self._fit_mode = FitModeFactory.create(new_type) - self._fit_mode_type = new_type - console.paragraph('Fit-mode type changed to') - console.print(new_type) - - def show_supported_fit_mode_types(self) -> None: # noqa: PLR6301 - """Print a table of supported fit-mode category types.""" - FitModeFactory.show_supported() + def show_supported_fit_mode_types(self) -> None: + """Print a table of supported fit modes for this project.""" + num_expts = len(self.project.experiments) if self.project.experiments else 0 + if num_expts <= 1: + modes = [FitModeEnum.SINGLE] + else: + modes = [FitModeEnum.SINGLE, FitModeEnum.JOINT, FitModeEnum.SEQUENTIAL] + columns_data = [[mode.value, mode.description()] for mode in modes] + console.paragraph('Supported fit modes') + render_table( + columns_headers=['Mode', 'Description'], + columns_alignment=['left', 'left'], + columns_data=columns_data, + ) def show_current_fit_mode_type(self) -> None: - """Print the currently used fit-mode category type.""" - console.paragraph('Current fit-mode type') - console.print(self._fit_mode_type) + """Print the currently selected fit mode.""" + console.paragraph('Current fit mode') + console.print(self._fit_mode.mode.value) # ------------------------------------------------------------------ # Joint-fit experiments (category) @@ -649,7 +554,7 @@ def joint_fit_experiments(self) -> object: """Per-experiment weight collection for joint fitting.""" return self._joint_fit_experiments - def fit(self, verbosity: str | None = None) -> None: + def fit(self, verbosity: str | None = None, *, use_physical_limits: bool = False) -> None: """ Execute fitting for all experiments. @@ -660,7 +565,8 @@ def fit(self, verbosity: str | None = None) -> None: In 'single' mode, fits each experiment independently. In 'joint' mode, performs a simultaneous fit across experiments with - weights. + weights. If mode is 'sequential', logs an error directing the + user to :meth:`fit_sequential` instead. Sets :attr:`fit_results` on success, which can be accessed programmatically (e.g., @@ -673,11 +579,10 @@ def fit(self, verbosity: str | None = None) -> None: experiment progress, ``'short'`` for a one-row-per-experiment summary table, or ``'silent'`` for no output. When ``None``, uses ``project.verbosity``. - - Raises - ------ - NotImplementedError - If the fit mode is not ``'single'`` or ``'joint'``. + use_physical_limits : bool, default=False + When ``True``, fall back to physical limits from the value + spec for parameters whose ``fit_min``/``fit_max`` are + unbounded. """ verb = VerbosityEnum(verbosity if verbosity is not None else self.project.verbosity) @@ -699,12 +604,16 @@ def fit(self, verbosity: str | None = None) -> None: # Run the fitting process mode = FitModeEnum(self._fit_mode.mode.value) if mode is FitModeEnum.JOINT: - self._fit_joint(verb, structures, experiments) + self._fit_joint(verb, structures, experiments, use_physical_limits=use_physical_limits) elif mode is FitModeEnum.SINGLE: - self._fit_single(verb, structures, experiments) - else: - msg = f'Fit mode {mode.value} not implemented yet.' - raise NotImplementedError(msg) + self._fit_single( + verb, structures, experiments, use_physical_limits=use_physical_limits + ) + elif mode is FitModeEnum.SEQUENTIAL: + log.error( + "fit_mode is 'sequential'. Use fit_sequential(data_dir=...) instead of fit()." + ) + return # After fitting, save the project if self.project.info.path is not None: @@ -715,6 +624,8 @@ def _fit_joint( verb: VerbosityEnum, structures: object, experiments: object, + *, + use_physical_limits: bool, ) -> None: """ Run joint fitting across all experiments with weights. @@ -727,6 +638,8 @@ def _fit_joint( Project structures collection. experiments : object Project experiments collection. + use_physical_limits : bool + Whether to use physical limits as fit bounds. """ mode = FitModeEnum.JOINT # Auto-populate joint_fit_experiments if empty @@ -749,6 +662,7 @@ def _fit_joint( weights=weights_array, analysis=self, verbosity=verb, + use_physical_limits=use_physical_limits, ) # After fitting, get the results @@ -759,6 +673,8 @@ def _fit_single( verb: VerbosityEnum, structures: object, experiments: object, + *, + use_physical_limits: bool, ) -> None: """ Run single-mode fitting for each experiment independently. @@ -771,6 +687,8 @@ def _fit_single( Project structures collection. experiments : object Project experiments collection. + use_physical_limits : bool + Whether to use physical limits as fit bounds. """ mode = FitModeEnum.SINGLE expt_names = experiments.names @@ -788,6 +706,7 @@ def _fit_single( [experiment], analysis=self, verbosity=verb, + use_physical_limits=use_physical_limits, ) # After fitting, snapshot parameter values before @@ -945,6 +864,9 @@ def fit_sequential( """ from easydiffraction.analysis.sequential import fit_sequential as _fit_seq # noqa: PLC0415 + # Record the fit mode for CIF serialization + self._fit_mode.mode = FitModeEnum.SEQUENTIAL.value + # Apply constraints before building the template self._update_categories() diff --git a/src/easydiffraction/analysis/categories/fit_mode/enums.py b/src/easydiffraction/analysis/categories/fit_mode/enums.py index 80b8d1def..6c12dbe80 100644 --- a/src/easydiffraction/analysis/categories/fit_mode/enums.py +++ b/src/easydiffraction/analysis/categories/fit_mode/enums.py @@ -12,6 +12,7 @@ class FitModeEnum(StrEnum): SINGLE = 'single' JOINT = 'joint' + SEQUENTIAL = 'sequential' @classmethod def default(cls) -> FitModeEnum: @@ -21,7 +22,9 @@ def default(cls) -> FitModeEnum: def description(self) -> str: """Return a human-readable description of this fit mode.""" if self is FitModeEnum.SINGLE: - return 'Independent fitting of each experiment; no shared parameters' + return 'Independent fitting of each experiment' if self is FitModeEnum.JOINT: - return 'Simultaneous fitting of all experiments; some parameters are shared' + return 'Simultaneous fitting of all experiments with weights' + if self is FitModeEnum.SEQUENTIAL: + return 'Sequential fitting over data files in a directory' return None diff --git a/src/easydiffraction/analysis/categories/fit_mode/fit_mode.py b/src/easydiffraction/analysis/categories/fit_mode/fit_mode.py index b0589a2e7..92c87f00a 100644 --- a/src/easydiffraction/analysis/categories/fit_mode/fit_mode.py +++ b/src/easydiffraction/analysis/categories/fit_mode/fit_mode.py @@ -25,7 +25,8 @@ class FitMode(CategoryItem): Fitting strategy selector. Holds a single ``mode`` descriptor whose value is one of - ``FitModeEnum`` members (``'single'`` or ``'joint'``). + ``FitModeEnum`` members (``'single'``, ``'joint'``, or + ``'sequential'``). """ type_info = TypeInfo( diff --git a/src/easydiffraction/analysis/fit_helpers/reporting.py b/src/easydiffraction/analysis/fit_helpers/reporting.py index f11cd7b18..6363c1e26 100644 --- a/src/easydiffraction/analysis/fit_helpers/reporting.py +++ b/src/easydiffraction/analysis/fit_helpers/reporting.py @@ -150,6 +150,42 @@ def display_results( columns_data=rows, ) + self._print_table_notes() + + def _print_table_notes(self) -> None: + """Print color-coded notes below the fitted parameters table.""" + notes: list[str] = [] + if any(getattr(p, '_outside_physical_limits', False) for p in self.parameters): + notes.append('[red]red fitted value[/red] — outside expected physical limits') + if any(_is_uncertainty_large(p) for p in self.parameters): + notes.append( + '[red]red uncertainty[/red] — exceeds the fitted value (poorly constrained)' + ) + for note in notes: + console.print(note) + + +def _is_uncertainty_large(param: object) -> bool: + """ + Return True when the uncertainty exceeds the absolute fitted value. + + Parameters + ---------- + param : object + Fitted parameter descriptor. + + Returns + ------- + bool + Whether the parameter is poorly constrained. + """ + if param.uncertainty is None or param.value is None: + return False + abs_value = abs(param.value) + if abs_value == 0: + return param.uncertainty > 0 + return param.uncertainty > abs_value + def _build_parameter_row(param: object) -> list[str]: """ @@ -168,7 +204,11 @@ def _build_parameter_row(param: object) -> list[str]: name = getattr(param, 'name', 'N/A') start = f'{param._fit_start_value:.4f}' if param._fit_start_value is not None else 'N/A' fitted = f'{param.value:.4f}' if param.value is not None else 'N/A' + if getattr(param, '_outside_physical_limits', False): + fitted = f'[red]{fitted}[/red]' uncertainty = f'{param.uncertainty:.4f}' if param.uncertainty is not None else 'N/A' + if _is_uncertainty_large(param): + uncertainty = f'[red]{uncertainty}[/red]' units = getattr(param, 'units', 'N/A') relative_change = _compute_relative_change(param) return [ diff --git a/src/easydiffraction/analysis/fitting.py b/src/easydiffraction/analysis/fitting.py index 4050d1e52..2b6e11ecd 100644 --- a/src/easydiffraction/analysis/fitting.py +++ b/src/easydiffraction/analysis/fitting.py @@ -9,6 +9,7 @@ import numpy as np from easydiffraction.analysis.fit_helpers.metrics import get_reliability_inputs +from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum from easydiffraction.analysis.minimizers.factory import MinimizerFactory from easydiffraction.core.variable import Parameter from easydiffraction.utils.enums import VerbosityEnum @@ -22,7 +23,7 @@ class Fitter: """Handles the fitting workflow using a pluggable minimizer.""" - def __init__(self, selection: str = 'lmfit') -> None: + def __init__(self, selection: str = MinimizerTypeEnum.default()) -> None: self.selection: str = selection self.engine: str = selection self.minimizer = MinimizerFactory.create(selection) @@ -35,6 +36,8 @@ def fit( weights: np.ndarray | None = None, analysis: object = None, verbosity: VerbosityEnum = VerbosityEnum.FULL, + *, + use_physical_limits: bool = False, ) -> None: """ Run the fitting process. @@ -57,6 +60,10 @@ def fit( fitting. verbosity : VerbosityEnum, default=VerbosityEnum.FULL Console output verbosity. + use_physical_limits : bool, default=False + When ``True``, fall back to physical limits from the value + spec for parameters whose ``fit_min``/``fit_max`` are + unbounded. """ expt_free_params: list[Parameter] = [] for expt in experiments: @@ -98,7 +105,12 @@ def objective_function(engine_params: dict[str, Any]) -> np.ndarray: ) # Perform fitting - self.results = self.minimizer.fit(params, objective_function, verbosity=verbosity) + self.results = self.minimizer.fit( + params, + objective_function, + verbosity=verbosity, + use_physical_limits=use_physical_limits, + ) def _process_fit_results( self, diff --git a/src/easydiffraction/analysis/minimizers/__init__.py b/src/easydiffraction/analysis/minimizers/__init__.py index 01fbc1364..7dddb75ee 100644 --- a/src/easydiffraction/analysis/minimizers/__init__.py +++ b/src/easydiffraction/analysis/minimizers/__init__.py @@ -1,5 +1,11 @@ # SPDX-FileCopyrightText: 2025 EasyScience contributors # SPDX-License-Identifier: BSD-3-Clause +from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer +from easydiffraction.analysis.minimizers.bumps_amoeba import BumpsAmoebaMinimizer +from easydiffraction.analysis.minimizers.bumps_de import BumpsDEMinimizer +from easydiffraction.analysis.minimizers.bumps_lm import BumpsLmMinimizer from easydiffraction.analysis.minimizers.dfols import DfolsMinimizer from easydiffraction.analysis.minimizers.lmfit import LmfitMinimizer +from easydiffraction.analysis.minimizers.lmfit_least_squares import LmfitLeastSquaresMinimizer +from easydiffraction.analysis.minimizers.lmfit_leastsq import LmfitLeastsqMinimizer diff --git a/src/easydiffraction/analysis/minimizers/base.py b/src/easydiffraction/analysis/minimizers/base.py index ee08ee55f..b3c7545ad 100644 --- a/src/easydiffraction/analysis/minimizers/base.py +++ b/src/easydiffraction/analysis/minimizers/base.py @@ -11,6 +11,9 @@ from easydiffraction.analysis.fit_helpers.reporting import FitResults from easydiffraction.analysis.fit_helpers.tracking import FitProgressTracker from easydiffraction.utils.enums import VerbosityEnum +from easydiffraction.utils.logging import log + +BOUNDARY_PROXIMITY_FRACTION = 0.01 class MinimizerBase(ABC): @@ -118,6 +121,8 @@ def _finalize_fit( Aggregated outcome of the fit. """ self._sync_result_to_parameters(parameters, raw_result) + self._warn_boundary_parameters(parameters) + self._warn_physical_limit_violations(parameters) success = self._check_success(raw_result) self.result = FitResults( success=success, @@ -129,6 +134,99 @@ def _finalize_fit( ) return self.result + @staticmethod + def _warn_boundary_parameters(parameters: list[object]) -> None: + """ + Warn if any parameter is near its fit bounds after fitting. + """ + for param in parameters: + v = param.value + lo, hi = param.fit_min, param.fit_max + span = hi - lo + if not np.isfinite(span): + # One-sided or unbounded — check absolute proximity + if np.isfinite(lo) and abs(v - lo) < BOUNDARY_PROXIMITY_FRACTION * max( + abs(lo), 1.0 + ): + log.warning( + f"Parameter '{param.unique_name}' ({v}) is at its lower " + f'bound ({lo}). Consider widening fit_min.' + ) + if np.isfinite(hi) and abs(v - hi) < BOUNDARY_PROXIMITY_FRACTION * max( + abs(hi), 1.0 + ): + log.warning( + f"Parameter '{param.unique_name}' ({v}) is at its upper " + f'bound ({hi}). Consider widening fit_max.' + ) + elif span > 0: + tol = BOUNDARY_PROXIMITY_FRACTION * span + if (v - lo) < tol: + log.warning( + f"Parameter '{param.unique_name}' ({v}) is at its lower " + f'bound ({lo}). Consider widening fit_min.' + ) + if (hi - v) < tol: + log.warning( + f"Parameter '{param.unique_name}' ({v}) is at its upper " + f'bound ({hi}). Consider widening fit_max.' + ) + + @staticmethod + def _apply_physical_limits(parameters: list[object]) -> None: + """ + Set fit bounds from physical limits for unbounded parameters. + + For each parameter whose ``fit_min`` is ``-inf``, replace it + with the lower physical limit from the value spec. Likewise for + ``fit_max`` and the upper physical limit. + + Parameters + ---------- + parameters : list[object] + Free parameters to adjust. + """ + for param in parameters: + if param.fit_min == -np.inf: + phys_lo = param._physical_lower_bound() + if np.isfinite(phys_lo): + param.fit_min = phys_lo + if param.fit_max == np.inf: + phys_hi = param._physical_upper_bound() + if np.isfinite(phys_hi): + param.fit_max = phys_hi + + @staticmethod + def _warn_physical_limit_violations(parameters: list[object]) -> None: + """ + Flag parameters outside their physical limits. + + Sets ``param._outside_physical_limits = True`` on any parameter + whose value falls outside its physical bounds. + + Parameters + ---------- + parameters : list[object] + Parameters after fitting. + """ + for param in parameters: + lo = param._physical_lower_bound() + hi = param._physical_upper_bound() + outside = False + if np.isfinite(lo) and param.value < lo: + log.warning( + f"Parameter '{param.unique_name}' ({param.value}) is below " + f'its physical lower limit ({lo}).' + ) + outside = True + if np.isfinite(hi) and param.value > hi: + log.warning( + f"Parameter '{param.unique_name}' ({param.value}) is above " + f'its physical upper limit ({hi}).' + ) + outside = True + param._outside_physical_limits = outside + @abstractmethod def _check_success(self, raw_result: object) -> bool: """Determine whether the fit was successful.""" @@ -138,6 +236,8 @@ def fit( parameters: list[object], objective_function: Callable[..., object], verbosity: VerbosityEnum = VerbosityEnum.FULL, + *, + use_physical_limits: bool = False, ) -> FitResults: """ Run the full minimization workflow. @@ -151,14 +251,21 @@ def fit( arguments. verbosity : VerbosityEnum, default=VerbosityEnum.FULL Console output verbosity. + use_physical_limits : bool, default=False + When ``True``, fall back to physical limits from the value + spec for parameters whose ``fit_min``/``fit_max`` are + unbounded. Returns ------- FitResults FitResults with success flag, best chi2 and timing. """ + if use_physical_limits: + self._apply_physical_limits(parameters) + minimizer_name = self.name or 'Unnamed Minimizer' - if self.method is not None: + if self.method is not None and f'({self.method})' not in minimizer_name: minimizer_name += f' ({self.method})' self._start_tracking(minimizer_name, verbosity=verbosity) diff --git a/src/easydiffraction/analysis/minimizers/bumps.py b/src/easydiffraction/analysis/minimizers/bumps.py new file mode 100644 index 000000000..109262343 --- /dev/null +++ b/src/easydiffraction/analysis/minimizers/bumps.py @@ -0,0 +1,266 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause +"""Minimizer using the bumps package.""" + +from __future__ import annotations + +import numpy as np +from bumps.fitproblem import FitProblem +from bumps.fitters import FITTERS +from bumps.fitters import FitDriver +from bumps.parameter import Parameter as BumpsParameter +from scipy.optimize import OptimizeResult + +from easydiffraction.analysis.minimizers.base import MinimizerBase +from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum +from easydiffraction.analysis.minimizers.factory import MinimizerFactory +from easydiffraction.core.metadata import TypeInfo + +DEFAULT_METHOD = 'lm' +DEFAULT_MAX_ITERATIONS = 1000 + + +class _EasyDiffractionFitness: + """ + Adaptor wrapping an EasyDiffraction objective into bumps Fitness. + """ + + def __init__( + self, + bumps_params: list[BumpsParameter], + objective_function: object, + ) -> None: + self._bumps_params = bumps_params + self._objective_function = objective_function + self._numpoints = 0 + + def parameters(self) -> dict[str, BumpsParameter]: + """Return bumps parameters as a name-keyed dictionary.""" + return {p.name: p for p in self._bumps_params} + + def update(self) -> None: + """Signal that parameters have changed (no-op).""" + + def residuals(self) -> np.ndarray: + """Compute residuals using current bumps parameter values.""" + values = np.array([p.value for p in self._bumps_params]) + r = self._objective_function(values) + self._numpoints = len(r) + return r + + def nllf(self) -> float: + """ + Negative log-likelihood as half the sum of squared residuals. + """ + r = self.residuals() + return 0.5 * np.sum(r**2) + + def numpoints(self) -> int: + """Return the number of data points.""" + return self._numpoints + + +@MinimizerFactory.register +class BumpsMinimizer(MinimizerBase): + """Minimizer using the bumps package.""" + + type_info = TypeInfo( + tag=MinimizerTypeEnum.BUMPS, + description='Bumps library using the default Levenberg-Marquardt method', + ) + + def __init__( + self, + name: str = MinimizerTypeEnum.BUMPS, + method: str = DEFAULT_METHOD, + max_iterations: int = DEFAULT_MAX_ITERATIONS, + ) -> None: + super().__init__( + name=name, + method=method, + max_iterations=max_iterations, + ) + + def _prepare_solver_args( # noqa: PLR6301 + self, + parameters: list[object], + ) -> dict[str, object]: + """ + Prepare bumps parameters from EasyDiffraction parameters. + + Parameters + ---------- + parameters : list[object] + List of parameters to be optimized. + + Returns + ------- + dict[str, object] + Dictionary containing the bumps parameter list. + """ + bumps_params = [] + for param in parameters: + bp = BumpsParameter( + value=param.value, + name=param._minimizer_uid, + ) + lo = param.fit_min + hi = param.fit_max + bp.range(lo, hi) + bumps_params.append(bp) + return {'bumps_params': bumps_params} + + def _run_solver( + self, + objective_function: object, + **kwargs: object, + ) -> object: + """ + Run the bumps solver. + + Uses FitDriver directly instead of bumps.fitters.fit() to skip + the expensive post-fit stderr/Jacobian computation that would + trigger extra objective-function evaluations. + + Parameters + ---------- + objective_function : object + The objective function to minimize. + **kwargs : object + Additional arguments for the solver. + + Returns + ------- + object + A scipy OptimizeResult with the optimized values. + """ + bumps_params = kwargs.get('bumps_params') + fitness = _EasyDiffractionFitness(bumps_params, objective_function) + fitness.nllf() # pre-compute so numpoints() is valid + problem = FitProblem(fitness) + + fitclass = next(cls for cls in FITTERS if cls.id == self.method) + driver = FitDriver( + fitclass=fitclass, + problem=problem, + monitors=[], + steps=self.max_iterations, + ) + driver.clip() + x, fx = driver.fit() + + success = x is not None + if success: + problem.setp(x) + + # Read values back from bumps Parameters in our original order. + # FitProblem sorts parameters alphabetically, so x from + # driver.fit() uses that sorted order — not ours. + result_x = np.array([p.value for p in bumps_params]) + + covar, stderr = ( + self._compute_covariance(bumps_params, fitness) if success else (None, None) + ) + var_names = [p.name for p in bumps_params] + + return OptimizeResult( + x=result_x, + dx=stderr, + fun=fx, + success=success, + status=0 if success else -1, + message='successful termination' if success else 'fit failed', + covar=covar, + var_names=var_names, + ) + + def _compute_covariance( # noqa: PLR6301 + self, + bumps_params: list[BumpsParameter], + fitness: _EasyDiffractionFitness, + ) -> tuple[np.ndarray | None, np.ndarray | None]: + """ + Compute covariance matrix and standard errors from a Jacobian. + + Parameters + ---------- + bumps_params : list[BumpsParameter] + Bumps parameters at their optimal values. + fitness : _EasyDiffractionFitness + Fitness object used to evaluate residuals. + + Returns + ------- + tuple[np.ndarray | None, np.ndarray | None] + ``(covariance_matrix, standard_errors)`` or ``(None, None)`` + when the computation fails. + """ + r0 = fitness.residuals() + n_points = len(r0) + n_params = len(bumps_params) + if n_points <= n_params: + return None, None + + step = np.sqrt(np.finfo(float).eps) + jacobian = np.empty((n_points, n_params)) + for j in range(n_params): + orig = bumps_params[j].value + h = step * max(abs(orig), 1.0) + bumps_params[j].value = orig + h + jacobian[:, j] = (fitness.residuals() - r0) / h + bumps_params[j].value = orig + + chi2_reduced = np.sum(r0**2) / (n_points - n_params) + try: + cov = np.linalg.inv(jacobian.T @ jacobian) * chi2_reduced + except np.linalg.LinAlgError: + return None, None + + stderr = np.sqrt(np.abs(np.diag(cov))) + return cov, stderr + + def _sync_result_to_parameters( # noqa: PLR6301 + self, + parameters: list[object], + raw_result: object, + ) -> None: + """ + Synchronize the result from the solver to the parameters. + + Parameters + ---------- + parameters : list[object] + List of parameters being optimized. + raw_result : object + The result object returned by the solver, or a numpy array + during optimization. + """ + if hasattr(raw_result, 'x'): + values = raw_result.x + uncertainties = getattr(raw_result, 'dx', None) + else: + values = raw_result + uncertainties = None + + for i, param in enumerate(parameters): + param._set_value_from_minimizer(float(values[i])) + if uncertainties is not None and i < len(uncertainties): + param.uncertainty = float(uncertainties[i]) + else: + param.uncertainty = None + + def _check_success(self, raw_result: object) -> bool: # noqa: PLR6301 + """ + Determine success from bumps OptimizeResult. + + Parameters + ---------- + raw_result : object + The result object returned by the solver. + + Returns + ------- + bool + True if the optimization was successful. + """ + return getattr(raw_result, 'success', False) diff --git a/src/easydiffraction/analysis/minimizers/bumps_amoeba.py b/src/easydiffraction/analysis/minimizers/bumps_amoeba.py new file mode 100644 index 000000000..3b916e6d4 --- /dev/null +++ b/src/easydiffraction/analysis/minimizers/bumps_amoeba.py @@ -0,0 +1,35 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause +"""Bumps minimizer variant using the Nelder-Mead simplex method.""" + +from __future__ import annotations + +from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer +from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum +from easydiffraction.analysis.minimizers.factory import MinimizerFactory +from easydiffraction.core.metadata import TypeInfo + +DEFAULT_METHOD = 'amoeba' +DEFAULT_MAX_ITERATIONS = 1000 + + +@MinimizerFactory.register +class BumpsAmoebaMinimizer(BumpsMinimizer): + """Bumps minimizer using the Nelder-Mead simplex method.""" + + type_info = TypeInfo( + tag=MinimizerTypeEnum.BUMPS_AMOEBA, + description='Bumps library with Nelder-Mead simplex method', + ) + + def __init__( + self, + name: str = MinimizerTypeEnum.BUMPS_AMOEBA, + method: str = DEFAULT_METHOD, + max_iterations: int = DEFAULT_MAX_ITERATIONS, + ) -> None: + super().__init__( + name=name, + method=method, + max_iterations=max_iterations, + ) diff --git a/src/easydiffraction/analysis/minimizers/bumps_de.py b/src/easydiffraction/analysis/minimizers/bumps_de.py new file mode 100644 index 000000000..98e5ef202 --- /dev/null +++ b/src/easydiffraction/analysis/minimizers/bumps_de.py @@ -0,0 +1,35 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause +"""Bumps minimizer variant using the differential evolution method.""" + +from __future__ import annotations + +from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer +from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum +from easydiffraction.analysis.minimizers.factory import MinimizerFactory +from easydiffraction.core.metadata import TypeInfo + +DEFAULT_METHOD = 'de' +DEFAULT_MAX_ITERATIONS = 1000 + + +@MinimizerFactory.register +class BumpsDEMinimizer(BumpsMinimizer): + """Bumps minimizer using the differential evolution method.""" + + type_info = TypeInfo( + tag=MinimizerTypeEnum.BUMPS_DE, + description='Bumps library with differential evolution method', + ) + + def __init__( + self, + name: str = MinimizerTypeEnum.BUMPS_DE, + method: str = DEFAULT_METHOD, + max_iterations: int = DEFAULT_MAX_ITERATIONS, + ) -> None: + super().__init__( + name=name, + method=method, + max_iterations=max_iterations, + ) diff --git a/src/easydiffraction/analysis/minimizers/bumps_lm.py b/src/easydiffraction/analysis/minimizers/bumps_lm.py new file mode 100644 index 000000000..c24cab240 --- /dev/null +++ b/src/easydiffraction/analysis/minimizers/bumps_lm.py @@ -0,0 +1,37 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause +"""Bumps minimizer variant using the Levenberg-Marquardt method.""" + +from __future__ import annotations + +from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer +from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum +from easydiffraction.analysis.minimizers.factory import MinimizerFactory +from easydiffraction.core.metadata import TypeInfo + +DEFAULT_METHOD = 'lm' +DEFAULT_MAX_ITERATIONS = 1000 + + +@MinimizerFactory.register +class BumpsLmMinimizer(BumpsMinimizer): + """ + Bumps minimizer explicitly using the Levenberg-Marquardt method. + """ + + type_info = TypeInfo( + tag=MinimizerTypeEnum.BUMPS_LM, + description='Bumps library with Levenberg-Marquardt method', + ) + + def __init__( + self, + name: str = MinimizerTypeEnum.BUMPS_LM, + method: str = DEFAULT_METHOD, + max_iterations: int = DEFAULT_MAX_ITERATIONS, + ) -> None: + super().__init__( + name=name, + method=method, + max_iterations=max_iterations, + ) diff --git a/src/easydiffraction/analysis/minimizers/dfols.py b/src/easydiffraction/analysis/minimizers/dfols.py index 1177ee4e5..f5dd41394 100644 --- a/src/easydiffraction/analysis/minimizers/dfols.py +++ b/src/easydiffraction/analysis/minimizers/dfols.py @@ -6,6 +6,7 @@ from dfols import solve from easydiffraction.analysis.minimizers.base import MinimizerBase +from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum from easydiffraction.analysis.minimizers.factory import MinimizerFactory from easydiffraction.core.metadata import TypeInfo @@ -17,13 +18,13 @@ class DfolsMinimizer(MinimizerBase): """Minimizer using DFO-LS (derivative-free least-squares).""" type_info = TypeInfo( - tag='dfols', - description='DFO-LS derivative-free least-squares optimization', + tag=MinimizerTypeEnum.DFOLS, + description='DFO-LS library for derivative-free least-squares optimization', ) def __init__( self, - name: str = 'dfols', + name: str = MinimizerTypeEnum.DFOLS, max_iterations: int = DEFAULT_MAX_ITERATIONS, **kwargs: object, ) -> None: diff --git a/src/easydiffraction/analysis/minimizers/enums.py b/src/easydiffraction/analysis/minimizers/enums.py new file mode 100644 index 000000000..5ba4a0473 --- /dev/null +++ b/src/easydiffraction/analysis/minimizers/enums.py @@ -0,0 +1,51 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause +"""Enumerations for minimizer types.""" + +from __future__ import annotations + +from enum import StrEnum + + +class MinimizerTypeEnum(StrEnum): + """Supported minimizer types.""" + + LMFIT = 'lmfit' + LMFIT_LEASTSQ = 'lmfit (leastsq)' + LMFIT_LEAST_SQUARES = 'lmfit (least_squares)' + DFOLS = 'dfols' + BUMPS = 'bumps' + BUMPS_LM = 'bumps (lm)' + BUMPS_AMOEBA = 'bumps (amoeba)' + BUMPS_DE = 'bumps (de)' + + @classmethod + def default(cls) -> MinimizerTypeEnum: + """Return the default minimizer type.""" + return cls.BUMPS_LM + + def description(self) -> str: + """ + Return a human-readable description of this minimizer type. + """ + descriptions = { + MinimizerTypeEnum.LMFIT: ( + 'LMFIT library using the default Levenberg-Marquardt least squares method' + ), + MinimizerTypeEnum.LMFIT_LEASTSQ: ( + 'LMFIT library with Levenberg-Marquardt least squares method' + ), + MinimizerTypeEnum.LMFIT_LEAST_SQUARES: ( + "LMFIT library with SciPy's trust region reflective algorithm" + ), + MinimizerTypeEnum.DFOLS: ( + 'DFO-LS library for derivative-free least-squares optimization' + ), + MinimizerTypeEnum.BUMPS: ( + 'BUMPS library using the default Levenberg-Marquardt method' + ), + MinimizerTypeEnum.BUMPS_LM: ('BUMPS library with Levenberg-Marquardt method'), + MinimizerTypeEnum.BUMPS_AMOEBA: ('BUMPS library with Nelder-Mead simplex method'), + MinimizerTypeEnum.BUMPS_DE: ('BUMPS library with differential evolution method'), + } + return descriptions.get(self, '') diff --git a/src/easydiffraction/analysis/minimizers/factory.py b/src/easydiffraction/analysis/minimizers/factory.py index e14f21166..9bbcd8d23 100644 --- a/src/easydiffraction/analysis/minimizers/factory.py +++ b/src/easydiffraction/analysis/minimizers/factory.py @@ -6,6 +6,7 @@ from typing import ClassVar +from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum from easydiffraction.core.factory import FactoryBase @@ -13,5 +14,5 @@ class MinimizerFactory(FactoryBase): """Factory for creating minimizer instances.""" _default_rules: ClassVar[dict] = { - frozenset(): 'lmfit', + frozenset(): MinimizerTypeEnum.default(), } diff --git a/src/easydiffraction/analysis/minimizers/lmfit.py b/src/easydiffraction/analysis/minimizers/lmfit.py index 6b09ebbe4..651b79265 100644 --- a/src/easydiffraction/analysis/minimizers/lmfit.py +++ b/src/easydiffraction/analysis/minimizers/lmfit.py @@ -5,6 +5,7 @@ import lmfit from easydiffraction.analysis.minimizers.base import MinimizerBase +from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum from easydiffraction.analysis.minimizers.factory import MinimizerFactory from easydiffraction.core.metadata import TypeInfo @@ -17,13 +18,13 @@ class LmfitMinimizer(MinimizerBase): """Minimizer using the lmfit package.""" type_info = TypeInfo( - tag='lmfit', - description='LMFIT with Levenberg-Marquardt least squares', + tag=MinimizerTypeEnum.LMFIT, + description='LMFIT library using the default Levenberg-Marquardt least squares method', ) def __init__( self, - name: str = 'lmfit', + name: str = MinimizerTypeEnum.LMFIT, method: str = DEFAULT_METHOD, max_iterations: int = DEFAULT_MAX_ITERATIONS, ) -> None: diff --git a/src/easydiffraction/analysis/minimizers/lmfit_least_squares.py b/src/easydiffraction/analysis/minimizers/lmfit_least_squares.py new file mode 100644 index 000000000..4daa20dff --- /dev/null +++ b/src/easydiffraction/analysis/minimizers/lmfit_least_squares.py @@ -0,0 +1,37 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause +"""LMFIT minimizer variant using trust region reflective method.""" + +from __future__ import annotations + +from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum +from easydiffraction.analysis.minimizers.factory import MinimizerFactory +from easydiffraction.analysis.minimizers.lmfit import LmfitMinimizer +from easydiffraction.core.metadata import TypeInfo + +DEFAULT_METHOD = 'least_squares' +DEFAULT_MAX_ITERATIONS = 1000 + + +@MinimizerFactory.register +class LmfitLeastSquaresMinimizer(LmfitMinimizer): + """ + LMFIT minimizer using SciPy's trust region reflective algorithm. + """ + + type_info = TypeInfo( + tag=MinimizerTypeEnum.LMFIT_LEAST_SQUARES, + description="LMFIT library with SciPy's trust region reflective algorithm", + ) + + def __init__( + self, + name: str = MinimizerTypeEnum.LMFIT_LEAST_SQUARES, + method: str = DEFAULT_METHOD, + max_iterations: int = DEFAULT_MAX_ITERATIONS, + ) -> None: + super().__init__( + name=name, + method=method, + max_iterations=max_iterations, + ) diff --git a/src/easydiffraction/analysis/minimizers/lmfit_leastsq.py b/src/easydiffraction/analysis/minimizers/lmfit_leastsq.py new file mode 100644 index 000000000..8d2050487 --- /dev/null +++ b/src/easydiffraction/analysis/minimizers/lmfit_leastsq.py @@ -0,0 +1,39 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause +""" +LMFIT minimizer variant using the Levenberg-Marquardt (leastsq) method. +""" + +from __future__ import annotations + +from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum +from easydiffraction.analysis.minimizers.factory import MinimizerFactory +from easydiffraction.analysis.minimizers.lmfit import LmfitMinimizer +from easydiffraction.core.metadata import TypeInfo + +DEFAULT_METHOD = 'leastsq' +DEFAULT_MAX_ITERATIONS = 1000 + + +@MinimizerFactory.register +class LmfitLeastsqMinimizer(LmfitMinimizer): + """ + LMFIT minimizer explicitly using the Levenberg-Marquardt method. + """ + + type_info = TypeInfo( + tag=MinimizerTypeEnum.LMFIT_LEASTSQ, + description='LMFIT library with Levenberg-Marquardt least squares method', + ) + + def __init__( + self, + name: str = MinimizerTypeEnum.LMFIT_LEASTSQ, + method: str = DEFAULT_METHOD, + max_iterations: int = DEFAULT_MAX_ITERATIONS, + ) -> None: + super().__init__( + name=name, + method=method, + max_iterations=max_iterations, + ) diff --git a/src/easydiffraction/core/variable.py b/src/easydiffraction/core/variable.py index c13bf28d5..71dd587ec 100644 --- a/src/easydiffraction/core/variable.py +++ b/src/easydiffraction/core/variable.py @@ -284,6 +284,24 @@ def __init__( self._constrained_spec = self._BOOL_SPEC_TEMPLATE self._constrained = self._constrained_spec.default + def _physical_lower_bound(self) -> float: + """ + Return the lower physical limit from the value spec, or -inf. + """ + validator = getattr(self._value_spec, '_validator', None) + if isinstance(validator, RangeValidator): + return validator.ge + return -np.inf + + def _physical_upper_bound(self) -> float: + """ + Return the upper physical limit from the value spec, or inf. + """ + validator = getattr(self._value_spec, '_validator', None) + if isinstance(validator, RangeValidator): + return validator.le + return np.inf + def __str__(self) -> str: """Return string representation with uncertainty and free.""" s = GenericDescriptorBase.__str__(self) diff --git a/src/easydiffraction/datablocks/experiment/categories/instrument/cwl.py b/src/easydiffraction/datablocks/experiment/categories/instrument/cwl.py index 3a3628bda..292896931 100644 --- a/src/easydiffraction/datablocks/experiment/categories/instrument/cwl.py +++ b/src/easydiffraction/datablocks/experiment/categories/instrument/cwl.py @@ -28,7 +28,7 @@ def __init__(self) -> None: units='Å', value_spec=AttributeSpec( default=1.5406, - validator=RangeValidator(), + validator=RangeValidator(ge=0.0), ), cif_handler=CifHandler( names=[ diff --git a/src/easydiffraction/datablocks/experiment/categories/linked_phases/default.py b/src/easydiffraction/datablocks/experiment/categories/linked_phases/default.py index 97d03c66f..eb6047a79 100644 --- a/src/easydiffraction/datablocks/experiment/categories/linked_phases/default.py +++ b/src/easydiffraction/datablocks/experiment/categories/linked_phases/default.py @@ -40,7 +40,7 @@ def __init__(self) -> None: description='Scale factor of the linked phase.', value_spec=AttributeSpec( default=1.0, - validator=RangeValidator(), + validator=RangeValidator(ge=0.0), ), cif_handler=CifHandler(names=['_pd_phase_block.scale']), ) diff --git a/src/easydiffraction/datablocks/experiment/item/base.py b/src/easydiffraction/datablocks/experiment/item/base.py index 954d390a1..61a25ca81 100644 --- a/src/easydiffraction/datablocks/experiment/item/base.py +++ b/src/easydiffraction/datablocks/experiment/item/base.py @@ -77,7 +77,7 @@ def type(self) -> object: # TODO: Consider another name return self._type # ------------------------------------------------------------------ - # Diffrn conditions (switchable-category pattern) + # Diffrn conditions (read-only, single type) # ------------------------------------------------------------------ @property @@ -85,44 +85,6 @@ def diffrn(self) -> object: """Ambient conditions recorded during measurement.""" return self._diffrn - @property - def diffrn_type(self) -> str: - """Tag of the active diffraction conditions type.""" - return self._diffrn_type - - @diffrn_type.setter - def diffrn_type(self, new_type: str) -> None: - """ - Switch to a different diffraction conditions type. - - Parameters - ---------- - new_type : str - Diffrn conditions tag (e.g. ``'default'``). - """ - supported_tags = DiffrnFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported diffrn type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_diffrn_types()'", - ) - return - - self._diffrn = DiffrnFactory.create(new_type) - self._diffrn_type = new_type - console.paragraph(f"Diffrn type for experiment '{self.name}' changed to") - console.print(new_type) - - def show_supported_diffrn_types(self) -> None: # noqa: PLR6301 - """Print a table of supported diffraction conditions types.""" - DiffrnFactory.show_supported() - - def show_current_diffrn_type(self) -> None: - """Print the currently used diffraction conditions type.""" - console.paragraph('Current diffrn type') - console.print(self.diffrn_type) - def _restore_switchable_types(self, block: object) -> None: """ Restore switchable category types from a parsed CIF block. @@ -346,23 +308,22 @@ def extinction_type(self, new_type: str) -> None: f"For more information, use 'show_supported_extinction_types()'", ) return - self._extinction = ExtinctionFactory.create(new_type) self._extinction_type = new_type - console.paragraph(f"Extinction type for experiment '{self.name}' changed to") + console.paragraph('Extinction type changed to') console.print(new_type) def show_supported_extinction_types(self) -> None: # noqa: PLR6301 - """Print a table of supported extinction correction models.""" + """Print a table of supported extinction correction types.""" ExtinctionFactory.show_supported() def show_current_extinction_type(self) -> None: - """Print the currently used extinction correction model.""" + """Print the currently used extinction correction type.""" console.paragraph('Current extinction type') - console.print(self.extinction_type) + console.print(self._extinction_type) # ------------------------------------------------------------------ - # Linked crystal (switchable-category pattern) + # Linked crystal (read-only, single type) # ------------------------------------------------------------------ @property @@ -370,46 +331,8 @@ def linked_crystal(self) -> object: """Linked crystal model for this experiment.""" return self._linked_crystal - @property - def linked_crystal_type(self) -> str: - """Tag of the active linked-crystal reference type.""" - return self._linked_crystal_type - - @linked_crystal_type.setter - def linked_crystal_type(self, new_type: str) -> None: - """ - Switch to a different linked-crystal reference type. - - Parameters - ---------- - new_type : str - Linked-crystal tag (e.g. ``'default'``). - """ - supported_tags = LinkedCrystalFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported linked crystal type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_linked_crystal_types()'", - ) - return - - self._linked_crystal = LinkedCrystalFactory.create(new_type) - self._linked_crystal_type = new_type - console.paragraph(f"Linked crystal type for experiment '{self.name}' changed to") - console.print(new_type) - - def show_supported_linked_crystal_types(self) -> None: # noqa: PLR6301 - """Print a table of supported linked-crystal reference types.""" - LinkedCrystalFactory.show_supported() - - def show_current_linked_crystal_type(self) -> None: - """Print the currently used linked-crystal reference type.""" - console.paragraph('Current linked crystal type') - console.print(self.linked_crystal_type) - # ------------------------------------------------------------------ - # Instrument (switchable-category pattern) + # Instrument (fixed at creation) # ------------------------------------------------------------------ @property @@ -417,54 +340,8 @@ def instrument(self) -> object: """Active instrument model for this experiment.""" return self._instrument - @property - def instrument_type(self) -> str: - """Tag of the active instrument type.""" - return self._instrument_type - - @instrument_type.setter - def instrument_type(self, new_type: str) -> None: - """ - Switch to a different instrument type. - - Parameters - ---------- - new_type : str - Instrument tag (e.g. ``'cwl-sc'``). - """ - supported = InstrumentFactory.supported_for( - scattering_type=self.type.scattering_type.value, - beam_mode=self.type.beam_mode.value, - sample_form=self.type.sample_form.value, - ) - supported_tags = [k.type_info.tag for k in supported] - if new_type not in supported_tags: - log.warning( - f"Unsupported instrument type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_instrument_types()'", - ) - return - self._instrument = InstrumentFactory.create(new_type) - self._instrument_type = new_type - console.paragraph(f"Instrument type for experiment '{self.name}' changed to") - console.print(new_type) - - def show_supported_instrument_types(self) -> None: - """Print a table of supported instrument types.""" - InstrumentFactory.show_supported( - scattering_type=self.type.scattering_type.value, - beam_mode=self.type.beam_mode.value, - sample_form=self.type.sample_form.value, - ) - - def show_current_instrument_type(self) -> None: - """Print the currently used instrument type.""" - console.paragraph('Current instrument type') - console.print(self.instrument_type) - # ------------------------------------------------------------------ - # Data (switchable-category pattern) + # Data (fixed at creation) # ------------------------------------------------------------------ @property @@ -472,43 +349,6 @@ def data(self) -> object: """Data collection for this experiment.""" return self._data - @property - def data_type(self) -> str: - """Tag of the active data collection type.""" - return self._data_type - - @data_type.setter - def data_type(self, new_type: str) -> None: - """ - Switch to a different data collection type. - - Parameters - ---------- - new_type : str - Data tag (e.g. ``'bragg-sc'``). - """ - supported_tags = DataFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported data type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_data_types()'", - ) - return - self._data = DataFactory.create(new_type) - self._data_type = new_type - console.paragraph(f"Data type for experiment '{self.name}' changed to") - console.print(new_type) - - def show_supported_data_types(self) -> None: # noqa: PLR6301 - """Print a table of supported data collection types.""" - DataFactory.show_supported() - - def show_current_data_type(self) -> None: - """Print the currently used data collection type.""" - console.paragraph('Current data type') - console.print(self.data_type) - class PdExperimentBase(ExperimentBase): """Base class for all powder experiments.""" @@ -597,89 +437,13 @@ def linked_phases(self) -> object: """Collection of phases linked to this experiment.""" return self._linked_phases - @property - def linked_phases_type(self) -> str: - """Tag of the active linked-phases collection type.""" - return self._linked_phases_type - - @linked_phases_type.setter - def linked_phases_type(self, new_type: str) -> None: - """ - Switch to a different linked-phases collection type. - - Parameters - ---------- - new_type : str - Linked-phases tag (e.g. ``'default'``). - """ - supported_tags = LinkedPhasesFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported linked phases type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_linked_phases_types()'", - ) - return - - self._linked_phases = LinkedPhasesFactory.create(new_type) - self._linked_phases_type = new_type - console.paragraph(f"Linked phases type for experiment '{self.name}' changed to") - console.print(new_type) - - def show_supported_linked_phases_types(self) -> None: # noqa: PLR6301 - """Print a table of supported linked-phases collection types.""" - LinkedPhasesFactory.show_supported() - - def show_current_linked_phases_type(self) -> None: - """Print the currently used linked-phases collection type.""" - console.paragraph('Current linked phases type') - console.print(self.linked_phases_type) - @property def excluded_regions(self) -> object: """Collection of excluded regions for the x-grid.""" return self._excluded_regions - @property - def excluded_regions_type(self) -> str: - """Tag of the active excluded-regions collection type.""" - return self._excluded_regions_type - - @excluded_regions_type.setter - def excluded_regions_type(self, new_type: str) -> None: - """ - Switch to a different excluded-regions collection type. - - Parameters - ---------- - new_type : str - Excluded-regions tag (e.g. ``'default'``). - """ - supported_tags = ExcludedRegionsFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported excluded regions type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_excluded_regions_types()'", - ) - return - - self._excluded_regions = ExcludedRegionsFactory.create(new_type) - self._excluded_regions_type = new_type - console.paragraph(f"Excluded regions type for experiment '{self.name}' changed to") - console.print(new_type) - - def show_supported_excluded_regions_types(self) -> None: # noqa: PLR6301 - """Print a table of supported excluded-regions types.""" - ExcludedRegionsFactory.show_supported() - - def show_current_excluded_regions_type(self) -> None: - """Print the currently used excluded-regions collection type.""" - console.paragraph('Current excluded regions type') - console.print(self.excluded_regions_type) - # ------------------------------------------------------------------ - # Data (switchable-category pattern) + # Data (fixed at creation) # ------------------------------------------------------------------ @property @@ -687,43 +451,6 @@ def data(self) -> object: """Data collection for this experiment.""" return self._data - @property - def data_type(self) -> str: - """Tag of the active data collection type.""" - return self._data_type - - @data_type.setter - def data_type(self, new_type: str) -> None: - """ - Switch to a different data collection type. - - Parameters - ---------- - new_type : str - Data tag (e.g. ``'bragg-pd-cwl'``). - """ - supported_tags = DataFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported data type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_data_types()'", - ) - return - self._data = DataFactory.create(new_type) - self._data_type = new_type - console.paragraph(f"Data type for experiment '{self.name}' changed to") - console.print(new_type) - - def show_supported_data_types(self) -> None: # noqa: PLR6301 - """Print a table of supported data collection types.""" - DataFactory.show_supported() - - def show_current_data_type(self) -> None: - """Print the currently used data collection type.""" - console.paragraph('Current data type') - console.print(self.data_type) - @property def peak(self) -> object: """Peak category object with profile parameters and mixins.""" diff --git a/src/easydiffraction/datablocks/experiment/item/bragg_pd.py b/src/easydiffraction/datablocks/experiment/item/bragg_pd.py index 37b02d99c..87e1bee65 100644 --- a/src/easydiffraction/datablocks/experiment/item/bragg_pd.py +++ b/src/easydiffraction/datablocks/experiment/item/bragg_pd.py @@ -120,7 +120,7 @@ def _load_ascii_data_to_experiment( return len(x) # ------------------------------------------------------------------ - # Instrument (switchable-category pattern) + # Instrument (fixed at creation) # ------------------------------------------------------------------ @property @@ -128,52 +128,6 @@ def instrument(self) -> object: """Active instrument model for this experiment.""" return self._instrument - @property - def instrument_type(self) -> str: - """Tag of the active instrument type.""" - return self._instrument_type - - @instrument_type.setter - def instrument_type(self, new_type: str) -> None: - """ - Switch to a different instrument type. - - Parameters - ---------- - new_type : str - Instrument tag (e.g. ``'cwl-pd'``). - """ - supported = InstrumentFactory.supported_for( - scattering_type=self.type.scattering_type.value, - beam_mode=self.type.beam_mode.value, - sample_form=self.type.sample_form.value, - ) - supported_tags = [k.type_info.tag for k in supported] - if new_type not in supported_tags: - log.warning( - f"Unsupported instrument type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_instrument_types()'", - ) - return - self._instrument = InstrumentFactory.create(new_type) - self._instrument_type = new_type - console.paragraph(f"Instrument type for experiment '{self.name}' changed to") - console.print(new_type) - - def show_supported_instrument_types(self) -> None: - """Print a table of supported instrument types.""" - InstrumentFactory.show_supported( - scattering_type=self.type.scattering_type.value, - beam_mode=self.type.beam_mode.value, - sample_form=self.type.sample_form.value, - ) - - def show_current_instrument_type(self) -> None: - """Print the currently used instrument type.""" - console.paragraph('Current instrument type') - console.print(self.instrument_type) - # ------------------------------------------------------------------ # Background (switchable-category pattern) # ------------------------------------------------------------------ diff --git a/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py b/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py index 6bc85eee9..db528d968 100644 --- a/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py +++ b/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py @@ -106,7 +106,7 @@ def __init__(self) -> None: 'fraction of the site occupied by the atom type.', value_spec=AttributeSpec( default=1.0, - validator=RangeValidator(), + validator=RangeValidator(ge=0.0, le=1.0), ), cif_handler=CifHandler(names=['_atom_site.occupancy']), ) @@ -116,7 +116,7 @@ def __init__(self) -> None: units='Ų', value_spec=AttributeSpec( default=0.0, - validator=RangeValidator(ge=0.0), + validator=RangeValidator(ge=0.0, le=100.0), ), cif_handler=CifHandler(names=['_atom_site.B_iso_or_equiv']), ) diff --git a/src/easydiffraction/datablocks/structure/categories/cell/default.py b/src/easydiffraction/datablocks/structure/categories/cell/default.py index e08761c93..86675c512 100644 --- a/src/easydiffraction/datablocks/structure/categories/cell/default.py +++ b/src/easydiffraction/datablocks/structure/categories/cell/default.py @@ -38,7 +38,7 @@ def __init__(self) -> None: units='Å', value_spec=AttributeSpec( default=10.0, - validator=RangeValidator(ge=0, le=1000), + validator=RangeValidator(ge=0, le=30), ), cif_handler=CifHandler(names=['_cell.length_a']), ) @@ -48,7 +48,7 @@ def __init__(self) -> None: units='Å', value_spec=AttributeSpec( default=10.0, - validator=RangeValidator(ge=0, le=1000), + validator=RangeValidator(ge=0, le=30), ), cif_handler=CifHandler(names=['_cell.length_b']), ) @@ -58,7 +58,7 @@ def __init__(self) -> None: units='Å', value_spec=AttributeSpec( default=10.0, - validator=RangeValidator(ge=0, le=1000), + validator=RangeValidator(ge=0, le=30), ), cif_handler=CifHandler(names=['_cell.length_c']), ) diff --git a/src/easydiffraction/datablocks/structure/item/base.py b/src/easydiffraction/datablocks/structure/item/base.py index 19c38df53..633e0b22a 100644 --- a/src/easydiffraction/datablocks/structure/item/base.py +++ b/src/easydiffraction/datablocks/structure/item/base.py @@ -12,7 +12,6 @@ from easydiffraction.datablocks.structure.categories.space_group import SpaceGroup from easydiffraction.datablocks.structure.categories.space_group.factory import SpaceGroupFactory from easydiffraction.utils.logging import console -from easydiffraction.utils.logging import log from easydiffraction.utils.utils import render_cif @@ -64,7 +63,7 @@ def name(self, new: str) -> None: self._name = new # ------------------------------------------------------------------ - # Cell (switchable-category pattern) + # Cell (read-only, single type) # ------------------------------------------------------------------ @property @@ -85,45 +84,8 @@ def cell(self, new: Cell) -> None: """ self._cell = new - @property - def cell_type(self) -> str: - """Tag of the active unit-cell type.""" - return self._cell_type - - @cell_type.setter - def cell_type(self, new_type: str) -> None: - """ - Switch to a different unit-cell type. - - Parameters - ---------- - new_type : str - Cell tag (e.g. ``'default'``). - """ - supported_tags = CellFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported cell type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_cell_types()'", - ) - return - self._cell = CellFactory.create(new_type) - self._cell_type = new_type - console.paragraph(f"Cell type for structure '{self.name}' changed to") - console.print(new_type) - - def show_supported_cell_types(self) -> None: # noqa: PLR6301 - """Print a table of supported unit-cell types.""" - CellFactory.show_supported() - - def show_current_cell_type(self) -> None: - """Print the currently used unit-cell type.""" - console.paragraph('Current cell type') - console.print(self.cell_type) - # ------------------------------------------------------------------ - # Space group (switchable-category pattern) + # Space group (read-only, single type) # ------------------------------------------------------------------ @property @@ -144,45 +106,8 @@ def space_group(self, new: SpaceGroup) -> None: """ self._space_group = new - @property - def space_group_type(self) -> str: - """Tag of the active space-group type.""" - return self._space_group_type - - @space_group_type.setter - def space_group_type(self, new_type: str) -> None: - """ - Switch to a different space-group type. - - Parameters - ---------- - new_type : str - Space-group tag (e.g. ``'default'``). - """ - supported_tags = SpaceGroupFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported space group type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_space_group_types()'", - ) - return - self._space_group = SpaceGroupFactory.create(new_type) - self._space_group_type = new_type - console.paragraph(f"Space group type for structure '{self.name}' changed to") - console.print(new_type) - - def show_supported_space_group_types(self) -> None: # noqa: PLR6301 - """Print a table of supported space-group types.""" - SpaceGroupFactory.show_supported() - - def show_current_space_group_type(self) -> None: - """Print the currently used space-group type.""" - console.paragraph('Current space group type') - console.print(self.space_group_type) - # ------------------------------------------------------------------ - # Atom sites (switchable-category pattern) + # Atom sites (read-only, single type) # ------------------------------------------------------------------ @property @@ -203,43 +128,6 @@ def atom_sites(self, new: AtomSites) -> None: """ self._atom_sites = new - @property - def atom_sites_type(self) -> str: - """Tag of the active atom-sites collection type.""" - return self._atom_sites_type - - @atom_sites_type.setter - def atom_sites_type(self, new_type: str) -> None: - """ - Switch to a different atom-sites collection type. - - Parameters - ---------- - new_type : str - Atom-sites tag (e.g. ``'default'``). - """ - supported_tags = AtomSitesFactory.supported_tags() - if new_type not in supported_tags: - log.warning( - f"Unsupported atom sites type '{new_type}'. " - f'Supported: {supported_tags}. ' - f"For more information, use 'show_supported_atom_sites_types()'", - ) - return - self._atom_sites = AtomSitesFactory.create(new_type) - self._atom_sites_type = new_type - console.paragraph(f"Atom sites type for structure '{self.name}' changed to") - console.print(new_type) - - def show_supported_atom_sites_types(self) -> None: # noqa: PLR6301 - """Print a table of supported atom-sites collection types.""" - AtomSitesFactory.show_supported() - - def show_current_atom_sites_type(self) -> None: - """Print the currently used atom-sites collection type.""" - console.paragraph('Current atom sites type') - console.print(self.atom_sites_type) - # ------------------------------------------------------------------ # Public methods # ------------------------------------------------------------------ diff --git a/src/easydiffraction/display/plotting.py b/src/easydiffraction/display/plotting.py index bf610183c..11b61c7c7 100644 --- a/src/easydiffraction/display/plotting.py +++ b/src/easydiffraction/display/plotting.py @@ -942,7 +942,13 @@ def _format_correlation_table_dataframe( if should_blank: row_values.append('') else: - row_values.append(f'{float(value):>{cell_width}.{precision}f}') + fval = float(value) + text = f'{fval:>{cell_width}.{precision}f}' + if fval < 0: + text = f'[red]{text}[/red]' + elif fval > 0: + text = f'[blue]{text}[/blue]' + row_values.append(text) rows.append([label, *row_values]) df = pd.DataFrame(rows, columns=pd.MultiIndex.from_tuples(headers)) diff --git a/src/easydiffraction/display/tablers/pandas.py b/src/easydiffraction/display/tablers/pandas.py index b88a6d080..affa3ff8f 100644 --- a/src/easydiffraction/display/tablers/pandas.py +++ b/src/easydiffraction/display/tablers/pandas.py @@ -11,10 +11,14 @@ HTML = None display = None +import re + from easydiffraction.display.tablers.base import TableBackendBase from easydiffraction.utils.environment import can_use_ipython_display from easydiffraction.utils.logging import log +_RICH_COLOR_RE = re.compile(r'\[(\w+)\](.*?)\[/\1\]') + class PandasTableBackend(TableBackendBase): """Render tables using the pandas Styler in Jupyter environments.""" @@ -106,6 +110,42 @@ def _build_header_alignment_styles(df: object, alignments: object) -> list[dict] for column, align in zip(df.columns, alignments, strict=False) ] + @staticmethod + def _strip_rich_markup(df: object) -> tuple[object, object | None]: + """ + Strip Rich color markup and build a CSS style frame. + + Scans every cell for patterns like ``[red]text[/red]``. Matching + cells have the markup removed and a corresponding ``color: + `` CSS entry in the returned style frame. + + Parameters + ---------- + df : object + DataFrame whose string cells may contain Rich markup. + + Returns + ------- + tuple[object, object | None] + ``(clean_df, style_df)`` where *style_df* is ``None`` when + no markup was found. + """ + clean = df.copy() + styles = df.copy().astype(str) + found = False + for col in df.columns: + for idx in df.index: + val = str(df.at[idx, col]) + m = _RICH_COLOR_RE.fullmatch(val) + if m: + tag, text = m.groups() + clean.at[idx, col] = text + styles.at[idx, col] = f'color: {tag}' + found = True + else: + styles.at[idx, col] = '' + return clean, styles if found else None + def _apply_styling(self, df: object, alignments: object, color: str) -> object: """ Build a configured Styler with alignments and base styles. @@ -124,10 +164,14 @@ def _apply_styling(self, df: object, alignments: object, color: str) -> object: object A configured pandas Styler ready for display. """ + df, color_styles = self._strip_rich_markup(df) + table_styles = self._build_base_styles(color) header_alignment_styles = self._build_header_alignment_styles(df, alignments) styler = df.style.format(precision=self.FLOAT_PRECISION) + if color_styles is not None: + styler = styler.apply(lambda _: color_styles, axis=None) styler = styler.set_table_attributes('class="dataframe"') # For mkdocs-jupyter styler = styler.set_table_styles(table_styles + header_alignment_styles) diff --git a/tests/functional/test_switchable_categories.py b/tests/functional/test_switchable_categories.py index 9b1301520..f24c6232e 100644 --- a/tests/functional/test_switchable_categories.py +++ b/tests/functional/test_switchable_categories.py @@ -31,17 +31,17 @@ def _make_project_with_experiment(): class TestAnalysisSwitchableCategories: - def test_aliases_type_default(self): + def test_aliases_default(self): project = _make_project_with_experiment() - assert project.analysis.aliases_type is not None + assert project.analysis.aliases is not None - def test_constraints_type_default(self): + def test_constraints_default(self): project = _make_project_with_experiment() - assert project.analysis.constraints_type is not None + assert project.analysis.constraints is not None - def test_fit_mode_type_default(self): + def test_fit_mode_default(self): project = _make_project_with_experiment() - assert project.analysis.fit_mode_type is not None + assert project.analysis.fit_mode is not None def test_minimizer_default(self): project = _make_project_with_experiment() diff --git a/tests/integration/fitting/test_analysis_display.py b/tests/integration/fitting/test_analysis_display.py index 7c7b1da39..d01521067 100644 --- a/tests/integration/fitting/test_analysis_display.py +++ b/tests/integration/fitting/test_analysis_display.py @@ -68,24 +68,6 @@ def test_show_available_minimizers(lbco_fitted_project): Analysis.show_available_minimizers() -def test_show_supported_aliases_types(lbco_fitted_project): - project = lbco_fitted_project - project.analysis.show_supported_aliases_types() - project.analysis.show_current_aliases_type() - - -def test_show_supported_constraints_types(lbco_fitted_project): - project = lbco_fitted_project - project.analysis.show_supported_constraints_types() - project.analysis.show_current_constraints_type() - - -def test_show_supported_fit_mode_types(lbco_fitted_project): - project = lbco_fitted_project - project.analysis.show_supported_fit_mode_types() - project.analysis.show_current_fit_mode_type() - - def test_fit_results_attributes(lbco_fitted_project): project = lbco_fitted_project results = project.analysis.fit_results diff --git a/tests/integration/fitting/test_exploration_help.py b/tests/integration/fitting/test_exploration_help.py index ba41ff1be..dbb5fc724 100644 --- a/tests/integration/fitting/test_exploration_help.py +++ b/tests/integration/fitting/test_exploration_help.py @@ -46,17 +46,11 @@ def test_structure_switchable_category_types(lbco_fitted_project): project = lbco_fitted_project model = project.structures['lbco'] # Cell - model.show_supported_cell_types() - model.show_current_cell_type() - assert isinstance(model.cell_type, str) + assert model.cell is not None # Space group - model.show_supported_space_group_types() - model.show_current_space_group_type() - assert isinstance(model.space_group_type, str) + assert model.space_group is not None # Atom sites - model.show_supported_atom_sites_types() - model.show_current_atom_sites_type() - assert isinstance(model.atom_sites_type, str) + assert model.atom_sites is not None def test_experiment_help(lbco_fitted_project): @@ -83,9 +77,7 @@ def test_experiment_switchable_category_types(lbco_fitted_project): project = lbco_fitted_project expt = project.experiments['hrpt'] # Instrument - expt.show_supported_instrument_types() - expt.show_current_instrument_type() - assert isinstance(expt.instrument_type, str) + assert expt.instrument is not None # Background expt.show_supported_background_types() expt.show_current_background_type() @@ -95,17 +87,13 @@ def test_experiment_switchable_category_types(lbco_fitted_project): expt.show_current_peak_profile_type() assert isinstance(expt.peak_profile_type, str) # Linked phases - expt.show_supported_linked_phases_types() - expt.show_current_linked_phases_type() - assert isinstance(expt.linked_phases_type, str) + assert expt.linked_phases is not None # Calculator expt.show_supported_calculator_types() expt.show_current_calculator_type() assert isinstance(expt.calculator_type, str) # Diffrn - expt.show_supported_diffrn_types() - expt.show_current_diffrn_type() - assert isinstance(expt.diffrn_type, str) + assert expt.diffrn is not None def test_experiment_data_info(lbco_fitted_project): diff --git a/tests/integration/fitting/test_single-crystal-diffraction.py b/tests/integration/fitting/test_single-crystal-diffraction.py index f5273fdd7..df3818a76 100644 --- a/tests/integration/fitting/test_single-crystal-diffraction.py +++ b/tests/integration/fitting/test_single-crystal-diffraction.py @@ -82,7 +82,7 @@ def test_single_fit_neut_sc_tof_taurine() -> None: # Compare fit quality chi2 = project.analysis.fit_results.reduced_chi_square - assert chi2 == pytest.approx(expected=23.6, abs=0.1) + assert chi2 == pytest.approx(expected=23.5, abs=0.1) if __name__ == '__main__': diff --git a/tests/integration/scipp-analysis/dream/test_analyze_reduced_data.py b/tests/integration/scipp-analysis/dream/test_analyze_reduced_data.py index 605bc7e93..31496c4b2 100644 --- a/tests/integration/scipp-analysis/dream/test_analyze_reduced_data.py +++ b/tests/integration/scipp-analysis/dream/test_analyze_reduced_data.py @@ -210,4 +210,4 @@ def test_analyze_reduced_data__fit_quality( ) -> None: """Verify fit quality is reasonable (chi-square value).""" chi_square = fitted_project.analysis.fit_results.reduced_chi_square - assert chi_square == pytest.approx(16.8, abs=0.1) + assert chi_square == pytest.approx(16.3, abs=0.1) diff --git a/tests/unit/easydiffraction/analysis/minimizers/test_base.py b/tests/unit/easydiffraction/analysis/minimizers/test_base.py index bfe7240e2..23d385e7c 100644 --- a/tests/unit/easydiffraction/analysis/minimizers/test_base.py +++ b/tests/unit/easydiffraction/analysis/minimizers/test_base.py @@ -4,6 +4,22 @@ import numpy as np +class _DummyParam: + """Minimal stand-in for an EasyDiffraction parameter.""" + + def __init__(self, v): + self.value = v + self.fit_min = -np.inf + self.fit_max = np.inf + self.unique_name = f'param_{v}' + + def _physical_lower_bound(self): + return -np.inf + + def _physical_upper_bound(self): + return np.inf + + def test_module_import(): import easydiffraction.analysis.minimizers.base as MUT @@ -13,10 +29,6 @@ def test_module_import(): def test_minimizer_base_fit_flow_and_finalize(): from easydiffraction.analysis.minimizers.base import MinimizerBase - class DummyParam: - def __init__(self, v): - self.value = v - class DummyResult: def __init__(self, *, success=True): self.success = success @@ -57,7 +69,7 @@ def _compute_residuals( minim = DummyMinimizer() - params = [DummyParam(1.0), DummyParam(2.0)] + params = [_DummyParam(1.0), _DummyParam(2.0)] # Wrap minimizer's objective creator to simulate higher-level usage objective = minim._create_objective_function( diff --git a/tests/unit/easydiffraction/analysis/minimizers/test_bumps.py b/tests/unit/easydiffraction/analysis/minimizers/test_bumps.py new file mode 100644 index 000000000..060864c1e --- /dev/null +++ b/tests/unit/easydiffraction/analysis/minimizers/test_bumps.py @@ -0,0 +1,415 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause + +from __future__ import annotations + +import types +from unittest.mock import MagicMock +from unittest.mock import patch + +import numpy as np +import pytest + + +def test_module_import(): + import easydiffraction.analysis.minimizers.bumps as MUT + + assert MUT.__name__ == 'easydiffraction.analysis.minimizers.bumps' + + +def test_type_info(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + + assert BumpsMinimizer.type_info.tag == MinimizerTypeEnum.BUMPS + + +def test_default_method(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + assert m.method == 'lm' + + +def test_default_max_iterations(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + assert m.max_iterations == 1000 + + +def test_is_subclass_of_base(): + from easydiffraction.analysis.minimizers.base import MinimizerBase + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + assert issubclass(BumpsMinimizer, MinimizerBase) + + +# -- Helpers for fake parameters ----------------------------------------------- + + +class FakeParam: + """Minimal stand-in for an EasyDiffraction parameter.""" + + def __init__(self, uid, value, *, fit_min=-np.inf, fit_max=np.inf): + self._minimizer_uid = uid + self._value = value + self.free = True + self.fit_min = fit_min + self.fit_max = fit_max + self.uncertainty = None + self.unique_name = uid + + @property + def value(self): + return self._value + + @value.setter + def value(self, v): + self._value = v + + def _set_value_from_minimizer(self, v): + self._value = v + + def _physical_lower_bound(self): + return -np.inf + + def _physical_upper_bound(self): + return np.inf + + +# -- _prepare_solver_args tests ------------------------------------------------ + + +def test_prepare_solver_args_returns_bumps_params(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + params = [FakeParam('a', 1.0), FakeParam('b', 2.0)] + kwargs = m._prepare_solver_args(params) + + assert 'bumps_params' in kwargs + bp = kwargs['bumps_params'] + assert len(bp) == 2 + assert bp[0].name == 'a' + assert bp[0].value == 1.0 + assert bp[1].name == 'b' + assert bp[1].value == 2.0 + + +def test_prepare_solver_args_applies_fit_bounds(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + params = [FakeParam('x', 5.0, fit_min=0.0, fit_max=10.0)] + kwargs = m._prepare_solver_args(params) + bp = kwargs['bumps_params'][0] + assert bp.bounds == (0.0, 10.0) + + +def test_prepare_solver_args_does_not_use_physical_bounds(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + class PhysParam(FakeParam): + def _physical_lower_bound(self): + return 0.0 + + def _physical_upper_bound(self): + return 100.0 + + m = BumpsMinimizer() + params = [PhysParam('x', 5.0)] + kwargs = m._prepare_solver_args(params) + bp = kwargs['bumps_params'][0] + assert bp.bounds == (-np.inf, np.inf) + + +# -- _EasyDiffractionFitness tests -------------------------------------------- + + +def test_fitness_parameters(): + from easydiffraction.analysis.minimizers.bumps import _EasyDiffractionFitness + + bp1 = MagicMock(name='p1', value=1.0) + bp1.name = 'p1' + bp2 = MagicMock(name='p2', value=2.0) + bp2.name = 'p2' + + fitness = _EasyDiffractionFitness([bp1, bp2], lambda v: v) + pdict = fitness.parameters() + assert set(pdict.keys()) == {'p1', 'p2'} + + +def test_fitness_residuals(): + from bumps.parameter import Parameter as BumpsParameter + + from easydiffraction.analysis.minimizers.bumps import _EasyDiffractionFitness + + bp = BumpsParameter(value=3.0, name='a') + obj = lambda values: np.array([values[0] - 1.0]) # noqa: E731 + fitness = _EasyDiffractionFitness([bp], obj) + r = fitness.residuals() + np.testing.assert_array_almost_equal(r, [2.0]) + + +def test_fitness_nllf(): + from bumps.parameter import Parameter as BumpsParameter + + from easydiffraction.analysis.minimizers.bumps import _EasyDiffractionFitness + + bp = BumpsParameter(value=4.0, name='a') + obj = lambda values: np.array([2.0, 2.0]) # noqa: E731 + fitness = _EasyDiffractionFitness([bp], obj) + nllf = fitness.nllf() + assert nllf == pytest.approx(4.0) # 0.5 * (4 + 4) + + +def test_fitness_numpoints_after_nllf(): + from bumps.parameter import Parameter as BumpsParameter + + from easydiffraction.analysis.minimizers.bumps import _EasyDiffractionFitness + + bp = BumpsParameter(value=0.0, name='a') + obj = lambda values: np.array([1.0, 2.0, 3.0]) # noqa: E731 + fitness = _EasyDiffractionFitness([bp], obj) + assert fitness.numpoints() == 0 # before calling nllf + fitness.nllf() + assert fitness.numpoints() == 3 + + +def test_fitness_update_is_noop(): + from easydiffraction.analysis.minimizers.bumps import _EasyDiffractionFitness + + fitness = _EasyDiffractionFitness([], lambda v: np.array([])) + fitness.update() # should not raise + + +# -- _run_solver tests --------------------------------------------------------- + + +def test_run_solver_returns_optimize_result(): + from scipy.optimize import OptimizeResult + + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + + fake_x = np.array([1.5, 2.5]) + + fake_fitter = types.SimpleNamespace(id='lm') + + with ( + patch('easydiffraction.analysis.minimizers.bumps.FitDriver') as mock_driver_cls, + patch('easydiffraction.analysis.minimizers.bumps.FitProblem'), + patch('easydiffraction.analysis.minimizers.bumps.FITTERS', [fake_fitter]), + patch.object(m, '_compute_covariance', return_value=(None, None)), + ): + driver_instance = mock_driver_cls.return_value + driver_instance.fit.return_value = (fake_x, 0.5) + driver_instance.clip = MagicMock() + + from bumps.parameter import Parameter as BumpsParameter + + bp1 = BumpsParameter(value=1.0, name='a') + bp2 = BumpsParameter(value=2.0, name='b') + bp1.value = 1.5 + bp2.value = 2.5 + + res = m._run_solver( + lambda v: np.array([0.0, 0.0]), + bumps_params=[bp1, bp2], + ) + + assert isinstance(res, OptimizeResult) + assert res.success is True + np.testing.assert_array_almost_equal(res.x, [1.5, 2.5]) + + +def test_run_solver_failure(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + + fake_fitter = types.SimpleNamespace(id='lm') + + with ( + patch('easydiffraction.analysis.minimizers.bumps.FitDriver') as mock_driver_cls, + patch('easydiffraction.analysis.minimizers.bumps.FitProblem'), + patch('easydiffraction.analysis.minimizers.bumps.FITTERS', [fake_fitter]), + ): + driver_instance = mock_driver_cls.return_value + driver_instance.fit.return_value = (None, None) + driver_instance.clip = MagicMock() + + from bumps.parameter import Parameter as BumpsParameter + + bp = BumpsParameter(value=1.0, name='a') + res = m._run_solver( + lambda v: np.array([0.0]), + bumps_params=[bp], + ) + + assert res.success is False + assert res.status == -1 + + +# -- _sync_result_to_parameters tests ----------------------------------------- + + +def test_sync_result_to_parameters_with_optimize_result(): + from scipy.optimize import OptimizeResult + + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + params = [FakeParam('a', 1.0), FakeParam('b', 2.0)] + result = OptimizeResult( + x=np.array([10.0, 20.0]), + dx=np.array([0.1, 0.2]), + success=True, + ) + m._sync_result_to_parameters(params, result) + assert params[0].value == 10.0 + assert params[0].uncertainty == 0.1 + assert params[1].value == 20.0 + assert params[1].uncertainty == 0.2 + + +def test_sync_result_to_parameters_without_dx(): + from scipy.optimize import OptimizeResult + + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + params = [FakeParam('a', 1.0)] + result = OptimizeResult(x=np.array([5.0]), success=True) + m._sync_result_to_parameters(params, result) + assert params[0].value == 5.0 + assert params[0].uncertainty is None + + +def test_sync_result_to_parameters_converts_to_float(): + from scipy.optimize import OptimizeResult + + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + params = [FakeParam('a', 1.0)] + result = OptimizeResult( + x=np.array([np.float64(7.0)]), + dx=np.array([np.float64(0.3)]), + success=True, + ) + m._sync_result_to_parameters(params, result) + assert isinstance(params[0].value, float) + assert isinstance(params[0].uncertainty, float) + + +def test_sync_with_raw_array(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + params = [FakeParam('a', 1.0)] + m._sync_result_to_parameters(params, np.array([99.0])) + assert params[0].value == 99.0 + assert params[0].uncertainty is None + + +# -- _check_success tests ----------------------------------------------------- + + +def test_check_success_true(): + from scipy.optimize import OptimizeResult + + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + assert m._check_success(OptimizeResult(success=True)) is True + + +def test_check_success_false(): + from scipy.optimize import OptimizeResult + + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + assert m._check_success(OptimizeResult(success=False)) is False + + +def test_check_success_missing_attribute(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + assert m._check_success(object()) is False + + +# -- _compute_covariance tests ------------------------------------------------ + + +def test_compute_covariance_basic(): + from bumps.parameter import Parameter as BumpsParameter + + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + from easydiffraction.analysis.minimizers.bumps import _EasyDiffractionFitness + + m = BumpsMinimizer() + bp = BumpsParameter(value=2.0, name='a') + # Linear objective: residual = value - target + obj = lambda values: np.array([values[0] - 1.0] * 5) # noqa: E731 + fitness = _EasyDiffractionFitness([bp], obj) + fitness.nllf() + + cov, stderr = m._compute_covariance([bp], fitness) + assert cov is not None + assert stderr is not None + assert cov.shape == (1, 1) + assert len(stderr) == 1 + assert stderr[0] > 0 + + +def test_compute_covariance_underdetermined(): + from bumps.parameter import Parameter as BumpsParameter + + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + from easydiffraction.analysis.minimizers.bumps import _EasyDiffractionFitness + + m = BumpsMinimizer() + bp1 = BumpsParameter(value=1.0, name='a') + bp2 = BumpsParameter(value=2.0, name='b') + # Only 1 data point but 2 parameters → underdetermined + obj = lambda values: np.array([values[0] + values[1]]) # noqa: E731 + fitness = _EasyDiffractionFitness([bp1, bp2], obj) + fitness.nllf() + + cov, stderr = m._compute_covariance([bp1, bp2], fitness) + assert cov is None + assert stderr is None + + +# -- optimize result var_names and covar fields -------------------------------- + + +def test_run_solver_result_has_var_names(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + + m = BumpsMinimizer() + + fake_x = np.array([1.0]) + fake_fitter = types.SimpleNamespace(id='lm') + + with ( + patch('easydiffraction.analysis.minimizers.bumps.FitDriver') as mock_driver_cls, + patch('easydiffraction.analysis.minimizers.bumps.FitProblem'), + patch('easydiffraction.analysis.minimizers.bumps.FITTERS', [fake_fitter]), + patch.object(m, '_compute_covariance', return_value=(None, None)), + ): + driver_instance = mock_driver_cls.return_value + driver_instance.fit.return_value = (fake_x, 0.1) + driver_instance.clip = MagicMock() + + from bumps.parameter import Parameter as BumpsParameter + + bp = BumpsParameter(value=1.0, name='my_param') + res = m._run_solver(lambda v: np.array([0.0]), bumps_params=[bp]) + + assert res.var_names == ['my_param'] diff --git a/tests/unit/easydiffraction/analysis/minimizers/test_bumps_amoeba.py b/tests/unit/easydiffraction/analysis/minimizers/test_bumps_amoeba.py new file mode 100644 index 000000000..2042b743e --- /dev/null +++ b/tests/unit/easydiffraction/analysis/minimizers/test_bumps_amoeba.py @@ -0,0 +1,99 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause + +from __future__ import annotations + +import numpy as np + + +def test_module_import(): + import easydiffraction.analysis.minimizers.bumps_amoeba as MUT + + assert MUT.__name__ == 'easydiffraction.analysis.minimizers.bumps_amoeba' + + +def test_type_info(): + from easydiffraction.analysis.minimizers.bumps_amoeba import BumpsAmoebaMinimizer + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + + assert BumpsAmoebaMinimizer.type_info.tag == MinimizerTypeEnum.BUMPS_AMOEBA + + +def test_is_subclass_of_bumps(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + from easydiffraction.analysis.minimizers.bumps_amoeba import BumpsAmoebaMinimizer + + assert issubclass(BumpsAmoebaMinimizer, BumpsMinimizer) + + +def test_default_method(): + from easydiffraction.analysis.minimizers.bumps_amoeba import BumpsAmoebaMinimizer + + m = BumpsAmoebaMinimizer() + assert m.method == 'amoeba' + + +def test_default_max_iterations(): + from easydiffraction.analysis.minimizers.bumps_amoeba import BumpsAmoebaMinimizer + + m = BumpsAmoebaMinimizer() + assert m.max_iterations == 1000 + + +def test_default_name(): + from easydiffraction.analysis.minimizers.bumps_amoeba import BumpsAmoebaMinimizer + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + + m = BumpsAmoebaMinimizer() + assert m.name == MinimizerTypeEnum.BUMPS_AMOEBA + + +def test_prepare_and_sync(): + from scipy.optimize import OptimizeResult + + from easydiffraction.analysis.minimizers.bumps_amoeba import BumpsAmoebaMinimizer + + class P: + def __init__(self, name, value): + self._minimizer_uid = name + self._value = value + self.free = True + self.fit_min = -np.inf + self.fit_max = np.inf + self.uncertainty = None + self.unique_name = name + + @property + def value(self): + return self._value + + @value.setter + def value(self, v): + self._value = v + + def _set_value_from_minimizer(self, v): + self._value = v + + def _physical_lower_bound(self): + return -np.inf + + def _physical_upper_bound(self): + return np.inf + + m = BumpsAmoebaMinimizer() + params = [P('p1', 1.0)] + + # Prepare + kwargs = m._prepare_solver_args(params) + assert 'bumps_params' in kwargs + assert kwargs['bumps_params'][0].name == 'p1' + + # Sync + result = OptimizeResult( + x=np.array([10.0]), + dx=np.array([0.5]), + success=True, + ) + m._sync_result_to_parameters(params, result) + assert params[0].value == 10.0 + assert params[0].uncertainty == 0.5 diff --git a/tests/unit/easydiffraction/analysis/minimizers/test_bumps_de.py b/tests/unit/easydiffraction/analysis/minimizers/test_bumps_de.py new file mode 100644 index 000000000..0e2bc3013 --- /dev/null +++ b/tests/unit/easydiffraction/analysis/minimizers/test_bumps_de.py @@ -0,0 +1,99 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause + +from __future__ import annotations + +import numpy as np + + +def test_module_import(): + import easydiffraction.analysis.minimizers.bumps_de as MUT + + assert MUT.__name__ == 'easydiffraction.analysis.minimizers.bumps_de' + + +def test_type_info(): + from easydiffraction.analysis.minimizers.bumps_de import BumpsDEMinimizer + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + + assert BumpsDEMinimizer.type_info.tag == MinimizerTypeEnum.BUMPS_DE + + +def test_is_subclass_of_bumps(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + from easydiffraction.analysis.minimizers.bumps_de import BumpsDEMinimizer + + assert issubclass(BumpsDEMinimizer, BumpsMinimizer) + + +def test_default_method(): + from easydiffraction.analysis.minimizers.bumps_de import BumpsDEMinimizer + + m = BumpsDEMinimizer() + assert m.method == 'de' + + +def test_default_max_iterations(): + from easydiffraction.analysis.minimizers.bumps_de import BumpsDEMinimizer + + m = BumpsDEMinimizer() + assert m.max_iterations == 1000 + + +def test_default_name(): + from easydiffraction.analysis.minimizers.bumps_de import BumpsDEMinimizer + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + + m = BumpsDEMinimizer() + assert m.name == MinimizerTypeEnum.BUMPS_DE + + +def test_prepare_and_sync(): + from scipy.optimize import OptimizeResult + + from easydiffraction.analysis.minimizers.bumps_de import BumpsDEMinimizer + + class P: + def __init__(self, name, value): + self._minimizer_uid = name + self._value = value + self.free = True + self.fit_min = -np.inf + self.fit_max = np.inf + self.uncertainty = None + self.unique_name = name + + @property + def value(self): + return self._value + + @value.setter + def value(self, v): + self._value = v + + def _set_value_from_minimizer(self, v): + self._value = v + + def _physical_lower_bound(self): + return -np.inf + + def _physical_upper_bound(self): + return np.inf + + m = BumpsDEMinimizer() + params = [P('p1', 1.0)] + + # Prepare + kwargs = m._prepare_solver_args(params) + assert 'bumps_params' in kwargs + assert kwargs['bumps_params'][0].name == 'p1' + + # Sync + result = OptimizeResult( + x=np.array([10.0]), + dx=np.array([0.5]), + success=True, + ) + m._sync_result_to_parameters(params, result) + assert params[0].value == 10.0 + assert params[0].uncertainty == 0.5 diff --git a/tests/unit/easydiffraction/analysis/minimizers/test_bumps_lm.py b/tests/unit/easydiffraction/analysis/minimizers/test_bumps_lm.py new file mode 100644 index 000000000..1cdea6f15 --- /dev/null +++ b/tests/unit/easydiffraction/analysis/minimizers/test_bumps_lm.py @@ -0,0 +1,100 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause + +from __future__ import annotations + +import numpy as np + + +def test_module_import(): + import easydiffraction.analysis.minimizers.bumps_lm as MUT + + assert MUT.__name__ == 'easydiffraction.analysis.minimizers.bumps_lm' + + +def test_type_info(): + from easydiffraction.analysis.minimizers.bumps_lm import BumpsLmMinimizer + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + + assert BumpsLmMinimizer.type_info.tag == MinimizerTypeEnum.BUMPS_LM + + +def test_is_subclass_of_bumps(): + from easydiffraction.analysis.minimizers.bumps import BumpsMinimizer + from easydiffraction.analysis.minimizers.bumps_lm import BumpsLmMinimizer + + assert issubclass(BumpsLmMinimizer, BumpsMinimizer) + + +def test_default_method(): + from easydiffraction.analysis.minimizers.bumps_lm import BumpsLmMinimizer + + m = BumpsLmMinimizer() + assert m.method == 'lm' + + +def test_default_max_iterations(): + from easydiffraction.analysis.minimizers.bumps_lm import BumpsLmMinimizer + + m = BumpsLmMinimizer() + assert m.max_iterations == 1000 + + +def test_default_name(): + from easydiffraction.analysis.minimizers.bumps_lm import BumpsLmMinimizer + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + + m = BumpsLmMinimizer() + assert m.name == MinimizerTypeEnum.BUMPS_LM + + +def test_prepare_and_sync(): + + from scipy.optimize import OptimizeResult + + from easydiffraction.analysis.minimizers.bumps_lm import BumpsLmMinimizer + + class P: + def __init__(self, name, value): + self._minimizer_uid = name + self._value = value + self.free = True + self.fit_min = -np.inf + self.fit_max = np.inf + self.uncertainty = None + self.unique_name = name + + @property + def value(self): + return self._value + + @value.setter + def value(self, v): + self._value = v + + def _set_value_from_minimizer(self, v): + self._value = v + + def _physical_lower_bound(self): + return -np.inf + + def _physical_upper_bound(self): + return np.inf + + m = BumpsLmMinimizer() + params = [P('p1', 1.0)] + + # Prepare + kwargs = m._prepare_solver_args(params) + assert 'bumps_params' in kwargs + assert kwargs['bumps_params'][0].name == 'p1' + + # Sync + result = OptimizeResult( + x=np.array([10.0]), + dx=np.array([0.5]), + success=True, + ) + m._sync_result_to_parameters(params, result) + assert params[0].value == 10.0 + assert params[0].uncertainty == 0.5 diff --git a/tests/unit/easydiffraction/analysis/minimizers/test_enums.py b/tests/unit/easydiffraction/analysis/minimizers/test_enums.py new file mode 100644 index 000000000..d15cda2e6 --- /dev/null +++ b/tests/unit/easydiffraction/analysis/minimizers/test_enums.py @@ -0,0 +1,32 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause + + +def test_module_import(): + import easydiffraction.analysis.minimizers.enums as MUT + + assert MUT.__name__ == 'easydiffraction.analysis.minimizers.enums' + + +def test_enum_members(): + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + + assert MinimizerTypeEnum.LMFIT == 'lmfit' + assert MinimizerTypeEnum.LMFIT_LEASTSQ == 'lmfit (leastsq)' + assert MinimizerTypeEnum.LMFIT_LEAST_SQUARES == 'lmfit (least_squares)' + assert MinimizerTypeEnum.DFOLS == 'dfols' + + +def test_enum_default(): + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + + assert MinimizerTypeEnum.default() is MinimizerTypeEnum.BUMPS_LM + + +def test_enum_descriptions(): + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + + for member in MinimizerTypeEnum: + desc = member.description() + assert isinstance(desc, str) + assert len(desc) > 0 diff --git a/tests/unit/easydiffraction/analysis/minimizers/test_factory.py b/tests/unit/easydiffraction/analysis/minimizers/test_factory.py index 108876b90..7351d84b2 100644 --- a/tests/unit/easydiffraction/analysis/minimizers/test_factory.py +++ b/tests/unit/easydiffraction/analysis/minimizers/test_factory.py @@ -9,7 +9,11 @@ def test_minimizer_factory_list_and_show(capsys): lst = MinimizerFactory.supported_tags() assert isinstance(lst, list) - assert len(lst) >= 1 + assert len(lst) >= 4 + assert 'lmfit' in lst + assert 'lmfit (leastsq)' in lst + assert 'lmfit (least_squares)' in lst + assert 'dfols' in lst MinimizerFactory.show_supported() out = capsys.readouterr().out assert 'Supported types' in out @@ -53,3 +57,21 @@ def _check_success(self, raw_result): created = MinimizerFactory.create('custom-test') assert isinstance(created, Custom) + + +def test_minimizer_factory_create_lmfit_leastsq(): + from easydiffraction.analysis.minimizers.factory import MinimizerFactory + from easydiffraction.analysis.minimizers.lmfit_leastsq import LmfitLeastsqMinimizer + + m = MinimizerFactory.create('lmfit (leastsq)') + assert isinstance(m, LmfitLeastsqMinimizer) + assert m.method == 'leastsq' + + +def test_minimizer_factory_create_lmfit_least_squares(): + from easydiffraction.analysis.minimizers.factory import MinimizerFactory + from easydiffraction.analysis.minimizers.lmfit_least_squares import LmfitLeastSquaresMinimizer + + m = MinimizerFactory.create('lmfit (least_squares)') + assert isinstance(m, LmfitLeastSquaresMinimizer) + assert m.method == 'least_squares' diff --git a/tests/unit/easydiffraction/analysis/minimizers/test_lmfit_least_squares.py b/tests/unit/easydiffraction/analysis/minimizers/test_lmfit_least_squares.py new file mode 100644 index 000000000..562b7cc1c --- /dev/null +++ b/tests/unit/easydiffraction/analysis/minimizers/test_lmfit_least_squares.py @@ -0,0 +1,90 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause + +import collections +import types + +import numpy as np + + +def test_module_import(): + import easydiffraction.analysis.minimizers.lmfit_least_squares as MUT + + assert MUT.__name__ == 'easydiffraction.analysis.minimizers.lmfit_least_squares' + + +def test_type_info(): + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + from easydiffraction.analysis.minimizers.lmfit_least_squares import LmfitLeastSquaresMinimizer + + assert LmfitLeastSquaresMinimizer.type_info.tag == MinimizerTypeEnum.LMFIT_LEAST_SQUARES + + +def test_is_subclass_of_lmfit(): + from easydiffraction.analysis.minimizers.lmfit import LmfitMinimizer + from easydiffraction.analysis.minimizers.lmfit_least_squares import LmfitLeastSquaresMinimizer + + assert issubclass(LmfitLeastSquaresMinimizer, LmfitMinimizer) + + +def test_default_method(): + from easydiffraction.analysis.minimizers.lmfit_least_squares import LmfitLeastSquaresMinimizer + + m = LmfitLeastSquaresMinimizer() + assert m.method == 'least_squares' + + +def test_prepare_and_sync(monkeypatch): + from easydiffraction.analysis.minimizers.lmfit_least_squares import LmfitLeastSquaresMinimizer + + class P: + def __init__(self, name, value, *, free=True, lo=-np.inf, hi=np.inf): + self._minimizer_uid = name + self._value = value + self.free = free + self.fit_min = lo + self.fit_max = hi + self.uncertainty = None + + @property + def value(self): + return self._value + + @value.setter + def value(self, v): + self._value = v + + def _set_value_from_minimizer(self, v): + self._value = v + + class FakeParam: + def __init__(self, value, stderr=None): + self.value = value + self.stderr = stderr + + class FakeParams(collections.UserDict): + def add(self, name, value, vary, min, max): + self[name] = types.SimpleNamespace(value=value, vary=vary, min=min, max=max) + + class FakeResult: + def __init__(self): + self.params = {'p1': FakeParam(7.0, stderr=0.3)} + self.success = True + + import easydiffraction.analysis.minimizers.lmfit as lm + + monkeypatch.setattr( + lm, + 'lmfit', + types.SimpleNamespace(Parameters=FakeParams, minimize=lambda *a, **k: FakeResult()), + ) + + minim = LmfitLeastSquaresMinimizer() + params = [P('p1', 1.0)] + + kwargs = minim._prepare_solver_args(params) + res = minim._run_solver(lambda *a, **k: np.array([0.0]), **kwargs) + minim._sync_result_to_parameters(params, res) + + assert params[0].value == 7.0 + assert params[0].uncertainty == 0.3 diff --git a/tests/unit/easydiffraction/analysis/minimizers/test_lmfit_leastsq.py b/tests/unit/easydiffraction/analysis/minimizers/test_lmfit_leastsq.py new file mode 100644 index 000000000..c899122ca --- /dev/null +++ b/tests/unit/easydiffraction/analysis/minimizers/test_lmfit_leastsq.py @@ -0,0 +1,90 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause + +import collections +import types + +import numpy as np + + +def test_module_import(): + import easydiffraction.analysis.minimizers.lmfit_leastsq as MUT + + assert MUT.__name__ == 'easydiffraction.analysis.minimizers.lmfit_leastsq' + + +def test_type_info(): + from easydiffraction.analysis.minimizers.enums import MinimizerTypeEnum + from easydiffraction.analysis.minimizers.lmfit_leastsq import LmfitLeastsqMinimizer + + assert LmfitLeastsqMinimizer.type_info.tag == MinimizerTypeEnum.LMFIT_LEASTSQ + + +def test_is_subclass_of_lmfit(): + from easydiffraction.analysis.minimizers.lmfit import LmfitMinimizer + from easydiffraction.analysis.minimizers.lmfit_leastsq import LmfitLeastsqMinimizer + + assert issubclass(LmfitLeastsqMinimizer, LmfitMinimizer) + + +def test_default_method(): + from easydiffraction.analysis.minimizers.lmfit_leastsq import LmfitLeastsqMinimizer + + m = LmfitLeastsqMinimizer() + assert m.method == 'leastsq' + + +def test_prepare_and_sync(monkeypatch): + from easydiffraction.analysis.minimizers.lmfit_leastsq import LmfitLeastsqMinimizer + + class P: + def __init__(self, name, value, *, free=True, lo=-np.inf, hi=np.inf): + self._minimizer_uid = name + self._value = value + self.free = free + self.fit_min = lo + self.fit_max = hi + self.uncertainty = None + + @property + def value(self): + return self._value + + @value.setter + def value(self, v): + self._value = v + + def _set_value_from_minimizer(self, v): + self._value = v + + class FakeParam: + def __init__(self, value, stderr=None): + self.value = value + self.stderr = stderr + + class FakeParams(collections.UserDict): + def add(self, name, value, vary, min, max): + self[name] = types.SimpleNamespace(value=value, vary=vary, min=min, max=max) + + class FakeResult: + def __init__(self): + self.params = {'p1': FakeParam(10.0, stderr=0.5)} + self.success = True + + import easydiffraction.analysis.minimizers.lmfit as lm + + monkeypatch.setattr( + lm, + 'lmfit', + types.SimpleNamespace(Parameters=FakeParams, minimize=lambda *a, **k: FakeResult()), + ) + + minim = LmfitLeastsqMinimizer() + params = [P('p1', 1.0)] + + kwargs = minim._prepare_solver_args(params) + res = minim._run_solver(lambda *a, **k: np.array([0.0]), **kwargs) + minim._sync_result_to_parameters(params, res) + + assert params[0].value == 10.0 + assert params[0].uncertainty == 0.5 diff --git a/tests/unit/easydiffraction/analysis/test_analysis.py b/tests/unit/easydiffraction/analysis/test_analysis.py index 9eea433f5..8de0722d7 100644 --- a/tests/unit/easydiffraction/analysis/test_analysis.py +++ b/tests/unit/easydiffraction/analysis/test_analysis.py @@ -34,7 +34,7 @@ def test_show_current_minimizer_prints(capsys): a.show_current_minimizer() out = capsys.readouterr().out assert 'Current minimizer' in out - assert 'lmfit' in out + assert 'bumps (lm)' in out def test_fit_mode_category_and_joint_fit_experiments(monkeypatch, capsys): @@ -53,51 +53,6 @@ def test_fit_mode_category_and_joint_fit_experiments(monkeypatch, capsys): assert len(a.joint_fit_experiments) == 0 -def test_fit_mode_type_getter(capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project_with_names([])) - assert a.fit_mode_type == 'default' - - -def test_show_supported_fit_mode_types(capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project_with_names([])) - a.show_supported_fit_mode_types() - out = capsys.readouterr().out - assert 'default' in out - - -def test_show_current_fit_mode_type(capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project_with_names([])) - a.show_current_fit_mode_type() - out = capsys.readouterr().out - assert 'Current fit-mode type' in out - assert 'default' in out - - -def test_fit_mode_type_setter_valid(capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project_with_names([])) - a.fit_mode_type = 'default' - assert a.fit_mode_type == 'default' - - -def test_fit_mode_type_setter_invalid(capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project_with_names([])) - a.fit_mode_type = 'nonexistent' - out = capsys.readouterr().out - assert 'Unsupported' in out - # Type should remain unchanged - assert a.fit_mode_type == 'default' - - def test_analysis_help(capsys): from easydiffraction.analysis.analysis import Analysis diff --git a/tests/unit/easydiffraction/analysis/test_analysis_coverage.py b/tests/unit/easydiffraction/analysis/test_analysis_coverage.py index 3d5e13796..7823e1f72 100644 --- a/tests/unit/easydiffraction/analysis/test_analysis_coverage.py +++ b/tests/unit/easydiffraction/analysis/test_analysis_coverage.py @@ -33,100 +33,6 @@ class P: return P() -# ------------------------------------------------------------------ -# Aliases switchable-category pattern -# ------------------------------------------------------------------ - - -class TestAliasesType: - def test_getter_returns_default(self): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project()) - assert a.aliases_type == 'default' - - def test_setter_valid(self, capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project()) - a.aliases_type = 'default' - out = capsys.readouterr().out - assert 'Aliases type changed to' in out - - def test_setter_invalid(self, capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project()) - a.aliases_type = 'nonexistent' - out = capsys.readouterr().out - assert 'Unsupported' in out - assert a.aliases_type == 'default' - - def test_show_supported(self, capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project()) - a.show_supported_aliases_types() - out = capsys.readouterr().out - assert 'default' in out - - def test_show_current(self, capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project()) - a.show_current_aliases_type() - out = capsys.readouterr().out - assert 'Current aliases type' in out - assert 'default' in out - - -# ------------------------------------------------------------------ -# Constraints switchable-category pattern -# ------------------------------------------------------------------ - - -class TestConstraintsType: - def test_getter_returns_default(self): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project()) - assert a.constraints_type == 'default' - - def test_setter_valid(self, capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project()) - a.constraints_type = 'default' - out = capsys.readouterr().out - assert 'Constraints type changed to' in out - - def test_setter_invalid(self, capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project()) - a.constraints_type = 'nonexistent' - out = capsys.readouterr().out - assert 'Unsupported' in out - assert a.constraints_type == 'default' - - def test_show_supported(self, capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project()) - a.show_supported_constraints_types() - out = capsys.readouterr().out - assert 'default' in out - - def test_show_current(self, capsys): - from easydiffraction.analysis.analysis import Analysis - - a = Analysis(project=_make_project()) - a.show_current_constraints_type() - out = capsys.readouterr().out - assert 'Current constraints type' in out - assert 'default' in out - - # ------------------------------------------------------------------ # AnalysisDisplay.as_cif # ------------------------------------------------------------------ @@ -258,8 +164,8 @@ def test_setter_changes_minimizer(self, capsys): from easydiffraction.analysis.analysis import Analysis a = Analysis(project=_make_project()) - assert a.current_minimizer == 'lmfit' - a.current_minimizer = 'lmfit' + assert a.current_minimizer == 'bumps (lm)' + a.current_minimizer = 'bumps (lm)' out = capsys.readouterr().out assert 'Current minimizer changed to' in out diff --git a/tests/unit/easydiffraction/analysis/test_fitting.py b/tests/unit/easydiffraction/analysis/test_fitting.py index 60bb794e1..f63788c07 100644 --- a/tests/unit/easydiffraction/analysis/test_fitting.py +++ b/tests/unit/easydiffraction/analysis/test_fitting.py @@ -22,7 +22,7 @@ class DummyExperiment: class DummyMin: tracker = type('T', (), {'track': staticmethod(lambda a, b: a)})() - def fit(self, params, obj, verbosity=None): + def fit(self, params, obj, verbosity=None, **kwargs): return None f = Fitter() @@ -58,7 +58,7 @@ class MockFitResults: class DummyMin: tracker = type('T', (), {'track': staticmethod(lambda a, b: a)})() - def fit(self, params, obj, verbosity=None): + def fit(self, params, obj, verbosity=None, **kwargs): return MockFitResults() def _sync_result_to_parameters(self, params, engine_params): diff --git a/tests/unit/easydiffraction/datablocks/experiment/item/test_base_coverage.py b/tests/unit/easydiffraction/datablocks/experiment/item/test_base_coverage.py index ed9d29955..8b6d63eb5 100644 --- a/tests/unit/easydiffraction/datablocks/experiment/item/test_base_coverage.py +++ b/tests/unit/easydiffraction/datablocks/experiment/item/test_base_coverage.py @@ -60,25 +60,6 @@ class TestExperimentBaseDiffrn: def test_diffrn_defaults(self): ex = ConcreteBase(name='ex1', type=_mk_type_powder_cwl_bragg()) assert ex.diffrn is not None - assert isinstance(ex.diffrn_type, str) - - def test_diffrn_type_invalid(self): - ex = ConcreteBase(name='ex1', type=_mk_type_powder_cwl_bragg()) - old_type = ex.diffrn_type - ex.diffrn_type = 'nonexistent' - assert ex.diffrn_type == old_type - - def test_show_supported_diffrn_types(self, capsys): - ex = ConcreteBase(name='ex1', type=_mk_type_powder_cwl_bragg()) - ex.show_supported_diffrn_types() - out = capsys.readouterr().out - assert len(out) > 0 - - def test_show_current_diffrn_type(self, capsys): - ex = ConcreteBase(name='ex1', type=_mk_type_powder_cwl_bragg()) - ex.show_current_diffrn_type() - out = capsys.readouterr().out - assert ex.diffrn_type in out class TestExperimentBaseCalculator: @@ -135,75 +116,18 @@ class TestPdExperimentLinkedPhases: def test_linked_phases_defaults(self): ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) assert ex.linked_phases is not None - assert isinstance(ex.linked_phases_type, str) - - def test_linked_phases_type_invalid(self): - ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) - old_type = ex.linked_phases_type - ex.linked_phases_type = 'nonexistent' - assert ex.linked_phases_type == old_type - - def test_show_supported_linked_phases_types(self, capsys): - ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) - ex.show_supported_linked_phases_types() - out = capsys.readouterr().out - assert len(out) > 0 - - def test_show_current_linked_phases_type(self, capsys): - ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) - ex.show_current_linked_phases_type() - out = capsys.readouterr().out - assert ex.linked_phases_type in out class TestPdExperimentExcludedRegions: def test_excluded_regions_defaults(self): ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) assert ex.excluded_regions is not None - assert isinstance(ex.excluded_regions_type, str) - - def test_excluded_regions_type_invalid(self): - ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) - old_type = ex.excluded_regions_type - ex.excluded_regions_type = 'nonexistent' - assert ex.excluded_regions_type == old_type - - def test_show_supported_excluded_regions_types(self, capsys): - ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) - ex.show_supported_excluded_regions_types() - out = capsys.readouterr().out - assert len(out) > 0 - - def test_show_current_excluded_regions_type(self, capsys): - ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) - ex.show_current_excluded_regions_type() - out = capsys.readouterr().out - assert ex.excluded_regions_type in out class TestPdExperimentData: def test_data_defaults(self): ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) assert ex.data is not None - assert isinstance(ex.data_type, str) - - def test_data_type_invalid(self): - ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) - old_type = ex.data_type - ex.data_type = 'nonexistent' - assert ex.data_type == old_type - - def test_show_supported_data_types(self, capsys): - ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) - ex.show_supported_data_types() - out = capsys.readouterr().out - assert len(out) > 0 - - def test_show_current_data_type(self, capsys): - ex = ConcretePd(name='pd1', type=_mk_type_powder_cwl_bragg()) - ex.show_current_data_type() - out = capsys.readouterr().out - assert ex.data_type in out class TestPdExperimentPeak: diff --git a/tests/unit/easydiffraction/datablocks/experiment/item/test_bragg_sc_coverage.py b/tests/unit/easydiffraction/datablocks/experiment/item/test_bragg_sc_coverage.py index 69c86b0c0..b1bdf34f7 100644 --- a/tests/unit/easydiffraction/datablocks/experiment/item/test_bragg_sc_coverage.py +++ b/tests/unit/easydiffraction/datablocks/experiment/item/test_bragg_sc_coverage.py @@ -72,13 +72,10 @@ def test_switchable_categories(self): assert isinstance(ex.extinction_type, str) # linked crystal assert ex.linked_crystal is not None - assert isinstance(ex.linked_crystal_type, str) # instrument assert ex.instrument is not None - assert isinstance(ex.instrument_type, str) # data assert ex.data is not None - assert isinstance(ex.data_type, str) def test_extinction_type_invalid(self): ex = CwlScExperiment(name='cwl_sc', type=_mk_type_sc_cwl()) @@ -86,12 +83,6 @@ def test_extinction_type_invalid(self): ex.extinction_type = 'bogus' assert ex.extinction_type == old - def test_linked_crystal_type_invalid(self): - ex = CwlScExperiment(name='cwl_sc', type=_mk_type_sc_cwl()) - old = ex.linked_crystal_type - ex.linked_crystal_type = 'bogus' - assert ex.linked_crystal_type == old - def test_show_supported_extinction_types(self, capsys): ex = CwlScExperiment(name='cwl_sc', type=_mk_type_sc_cwl()) ex.show_supported_extinction_types() @@ -104,42 +95,6 @@ def test_show_current_extinction_type(self, capsys): out = capsys.readouterr().out assert ex.extinction_type in out - def test_show_supported_linked_crystal_types(self, capsys): - ex = CwlScExperiment(name='cwl_sc', type=_mk_type_sc_cwl()) - ex.show_supported_linked_crystal_types() - out = capsys.readouterr().out - assert len(out) > 0 - - def test_show_current_linked_crystal_type(self, capsys): - ex = CwlScExperiment(name='cwl_sc', type=_mk_type_sc_cwl()) - ex.show_current_linked_crystal_type() - out = capsys.readouterr().out - assert ex.linked_crystal_type in out - - def test_show_supported_instrument_types(self, capsys): - ex = CwlScExperiment(name='cwl_sc', type=_mk_type_sc_cwl()) - ex.show_supported_instrument_types() - out = capsys.readouterr().out - assert len(out) > 0 - - def test_show_current_instrument_type(self, capsys): - ex = CwlScExperiment(name='cwl_sc', type=_mk_type_sc_cwl()) - ex.show_current_instrument_type() - out = capsys.readouterr().out - assert ex.instrument_type in out - - def test_show_supported_data_types(self, capsys): - ex = CwlScExperiment(name='cwl_sc', type=_mk_type_sc_cwl()) - ex.show_supported_data_types() - out = capsys.readouterr().out - assert len(out) > 0 - - def test_show_current_data_type(self, capsys): - ex = CwlScExperiment(name='cwl_sc', type=_mk_type_sc_cwl()) - ex.show_current_data_type() - out = capsys.readouterr().out - assert ex.data_type in out - class TestTofScExperiment: def test_init(self): diff --git a/tests/unit/easydiffraction/datablocks/structure/item/test_base_coverage.py b/tests/unit/easydiffraction/datablocks/structure/item/test_base_coverage.py index a8c4b5fb7..6d8967703 100644 --- a/tests/unit/easydiffraction/datablocks/structure/item/test_base_coverage.py +++ b/tests/unit/easydiffraction/datablocks/structure/item/test_base_coverage.py @@ -43,124 +43,49 @@ def test_setter_type_check(self, structure): # ------------------------------------------------------------------ -# Cell (switchable-category) +# Cell (read-only, single type) # ------------------------------------------------------------------ class TestStructureCell: - def test_default_cell_type(self, structure): - assert structure.cell_type == CellFactory.default_tag() - def test_cell_returns_cell_instance(self, structure): assert isinstance(structure.cell, Cell) - def test_cell_type_setter_valid(self, structure, capsys): - supported = CellFactory.supported_tags() - assert len(supported) > 0 - tag = supported[0] - structure.cell_type = tag - assert structure.cell_type == tag - - def test_cell_type_setter_invalid_keeps_old(self, structure): - old_type = structure.cell_type - structure.cell_type = 'nonexistent-type' - assert structure.cell_type == old_type - def test_cell_setter_replaces_instance(self, structure): new_cell = CellFactory.create(CellFactory.default_tag()) structure.cell = new_cell assert structure.cell is new_cell - def test_show_supported_cell_types(self, structure, capsys): - structure.show_supported_cell_types() - out = capsys.readouterr().out - assert len(out) > 0 - - def test_show_current_cell_type(self, structure, capsys): - structure.show_current_cell_type() - out = capsys.readouterr().out - assert structure.cell_type in out - # ------------------------------------------------------------------ -# Space group (switchable-category) +# Space group (read-only, single type) # ------------------------------------------------------------------ class TestStructureSpaceGroup: - def test_default_space_group_type(self, structure): - assert structure.space_group_type == SpaceGroupFactory.default_tag() - def test_space_group_returns_instance(self, structure): assert isinstance(structure.space_group, SpaceGroup) - def test_space_group_type_setter_valid(self, structure, capsys): - supported = SpaceGroupFactory.supported_tags() - assert len(supported) > 0 - tag = supported[0] - structure.space_group_type = tag - assert structure.space_group_type == tag - - def test_space_group_type_setter_invalid_keeps_old(self, structure): - old_type = structure.space_group_type - structure.space_group_type = 'nonexistent-type' - assert structure.space_group_type == old_type - def test_space_group_setter_replaces_instance(self, structure): new_sg = SpaceGroupFactory.create(SpaceGroupFactory.default_tag()) structure.space_group = new_sg assert structure.space_group is new_sg - def test_show_supported_space_group_types(self, structure, capsys): - structure.show_supported_space_group_types() - out = capsys.readouterr().out - assert len(out) > 0 - - def test_show_current_space_group_type(self, structure, capsys): - structure.show_current_space_group_type() - out = capsys.readouterr().out - assert structure.space_group_type in out - # ------------------------------------------------------------------ -# Atom sites (switchable-category) +# Atom sites (read-only, single type) # ------------------------------------------------------------------ class TestStructureAtomSites: - def test_default_atom_sites_type(self, structure): - assert structure.atom_sites_type == AtomSitesFactory.default_tag() - def test_atom_sites_returns_instance(self, structure): assert isinstance(structure.atom_sites, AtomSites) - def test_atom_sites_type_setter_valid(self, structure, capsys): - supported = AtomSitesFactory.supported_tags() - assert len(supported) > 0 - tag = supported[0] - structure.atom_sites_type = tag - assert structure.atom_sites_type == tag - - def test_atom_sites_type_setter_invalid_keeps_old(self, structure): - old_type = structure.atom_sites_type - structure.atom_sites_type = 'nonexistent-type' - assert structure.atom_sites_type == old_type - def test_atom_sites_setter_replaces_instance(self, structure): new_as = AtomSitesFactory.create(AtomSitesFactory.default_tag()) structure.atom_sites = new_as assert structure.atom_sites is new_as - def test_show_supported_atom_sites_types(self, structure, capsys): - structure.show_supported_atom_sites_types() - out = capsys.readouterr().out - assert len(out) > 0 - - def test_show_current_atom_sites_type(self, structure, capsys): - structure.show_current_atom_sites_type() - out = capsys.readouterr().out - assert structure.atom_sites_type in out - # ------------------------------------------------------------------ # Display methods diff --git a/tests/unit/easydiffraction/display/test_plotting.py b/tests/unit/easydiffraction/display/test_plotting.py index 9ff1c5a04..a40b2ba1a 100644 --- a/tests/unit/easydiffraction/display/test_plotting.py +++ b/tests/unit/easydiffraction/display/test_plotting.py @@ -1,9 +1,16 @@ # SPDX-FileCopyrightText: 2026 EasyScience contributors # SPDX-License-Identifier: BSD-3-Clause +import re + import pytest +def _strip_markup(text: str) -> str: + """Remove Rich color markup tags like [red]...[/red].""" + return re.sub(r'\[(\w+)\](.*?)\[/\1\]', r'\2', text) + + def test_module_import(): import easydiffraction.display.plotting as MUT @@ -219,7 +226,7 @@ class Project: assert df.iloc[0, 0] == 'phase.cell.length_a' assert df.iloc[0, 1] == '' assert df.iloc[1, 0] == 'phase.cell.length_b' - assert df.iloc[1, 1].strip() == '0.167' + assert _strip_markup(df.iloc[1, 1]).strip() == '0.167' def test_plot_param_correlations_renders_plotly_heatmap(monkeypatch): @@ -420,7 +427,7 @@ class Project: assert df.iloc[0, 0] == 'phase.scale' assert df.iloc[0, 1] == '' assert df.iloc[1, 0] == 'phase.cell.length_a' - assert df.iloc[1, 1].strip() == '0.82' + assert _strip_markup(df.iloc[1, 1]).strip() == '0.82' def test_plot_param_correlations_hides_subthreshold_table_values(monkeypatch): @@ -489,11 +496,11 @@ class Project: assert df.iloc[0, 1] == '' assert df.iloc[1, 1] == '' assert df.iloc[2, 1] == '' - assert df.iloc[2, 2].strip() == '-0.91' + assert _strip_markup(df.iloc[2, 2]).strip() == '-0.91' assert df.iloc[3, 1] == '' assert df.iloc[3, 2] == '' - assert df.iloc[3, 3].strip() == '-0.89' - assert df.iloc[4, 1].strip() == '0.82' + assert _strip_markup(df.iloc[3, 3]).strip() == '-0.89' + assert _strip_markup(df.iloc[4, 1]).strip() == '0.82' assert df.iloc[4, 2] == '' assert df.iloc[4, 3] == '' assert df.iloc[4, 4] == '' diff --git a/tests/unit/easydiffraction/project/test_project_load.py b/tests/unit/easydiffraction/project/test_project_load.py index c676cb89b..807cb580b 100644 --- a/tests/unit/easydiffraction/project/test_project_load.py +++ b/tests/unit/easydiffraction/project/test_project_load.py @@ -70,7 +70,7 @@ def test_round_trips_minimizer(self, tmp_path): loaded = Project.load(str(tmp_path / 'proj')) - assert loaded.analysis.current_minimizer == 'lmfit' + assert loaded.analysis.current_minimizer == 'bumps (lm)' def test_round_trips_fit_mode(self, tmp_path): original = Project(name='a2') @@ -125,7 +125,7 @@ def test_loads_analysis_from_subdir(self, tmp_path): assert (tmp_path / 'proj' / 'analysis' / 'analysis.cif').is_file() loaded = Project.load(str(tmp_path / 'proj')) - assert loaded.analysis.current_minimizer == 'lmfit' + assert loaded.analysis.current_minimizer == 'bumps (lm)' def test_loads_analysis_from_root_fallback(self, tmp_path): """Old layout fallback: analysis.cif at project root.""" @@ -139,4 +139,4 @@ def test_loads_analysis_from_root_fallback(self, tmp_path): analysis_dir.rmdir() loaded = Project.load(str(proj_dir)) - assert loaded.analysis.current_minimizer == 'lmfit' + assert loaded.analysis.current_minimizer == 'bumps (lm)'