From fe49fd0bc20f94b428abb06220ae3dc2134b22c8 Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:48:53 -0400 Subject: [PATCH 01/10] feat(core): add the exceptions module and structured-warning shape PowderLineError base plus RecipeValidationError, GatewayNotInstalledError, EngineNotAvailableError (also an ImportError, A52), EngineVersionError (A44) and EngineExecutionError; StructuredWarning = {code, message, field_path}. Engine-free. The registry and gateways raise these from the next commits. Co-Authored-By: Claude Opus 5.5 --- src/powderline/exceptions.py | 105 +++++++++++++++++++++++++++++++++++ tests/test_exceptions.py | 77 +++++++++++++++++++++++++ 2 files changed, 182 insertions(+) create mode 100644 src/powderline/exceptions.py create mode 100644 tests/test_exceptions.py diff --git a/src/powderline/exceptions.py b/src/powderline/exceptions.py new file mode 100644 index 0000000..92455c2 --- /dev/null +++ b/src/powderline/exceptions.py @@ -0,0 +1,105 @@ +"""PowderLine exception hierarchy and the structured-warning shape (engine-free). + +Every PowderLine-raised error derives from :class:`PowderLineError`. The +gateway errors split along the schema/runtime layers (master plan A23, A5/A8): + +- :class:`GatewayNotInstalledError` — no gateway of that name is registered at + all (an out-of-tree plugin that is not installed). The only case where + ``validate()`` cannot run. +- :class:`EngineNotAvailableError` — the gateway is present but its engine + dependency is missing from this environment. Raised at ``run()`` time only, + never by ``validate()``/``validate_only``. Also an :class:`ImportError`, so + callers that caught the underlying import failure keep working (A52). +- :class:`EngineVersionError` — the installed engine is outside the gateway's + supported ``ENGINE_VERSION_SPEC``. A hard error (A44). +- :class:`EngineExecutionError` — the engine ran and failed. +- :class:`RecipeValidationError` — a recipe failed validation. + +This module must never import an engine (enforced by an import-block test). +""" + +from __future__ import annotations + +from typing import Optional, TypedDict + + +class PowderLineError(Exception): + """Base class for all PowderLine errors.""" + + +class RecipeValidationError(PowderLineError, ValueError): + """A recipe failed validation. + + Wraps (and may annotate) the underlying pydantic errors. ``errors`` holds + pydantic's ``ValidationError.errors()`` list when built from one. + """ + + def __init__(self, message: str, *, errors: Optional[list] = None, + schema_name: Optional[str] = None) -> None: + super().__init__(message) + self.errors = list(errors or []) + self.schema_name = schema_name + + @classmethod + def from_pydantic(cls, exc, *, schema_name: Optional[str] = None) -> "RecipeValidationError": + """Build from a ``pydantic.ValidationError``, keeping its error list.""" + prefix = f"{schema_name} recipe" if schema_name else "recipe" + err = cls(f"{prefix} failed validation:\n{exc}", errors=exc.errors(), + schema_name=schema_name) + err.__cause__ = exc + return err + + +class GatewayNotInstalledError(PowderLineError): + """No gateway of this name is registered (the gateway package is absent).""" + + def __init__(self, message: str, *, gateway: str) -> None: + super().__init__(message) + self.gateway = gateway + + +class EngineNotAvailableError(PowderLineError, ImportError): + """The gateway is present, but its engine dependency is not installed here.""" + + def __init__(self, *, gateway: str, dependency: str, env: str, + install_command: str) -> None: + super().__init__( + f"the {gateway!r} gateway needs {dependency}, which is not installed " + f"in this environment; use the {env!r} pixi environment " + f"(install it with: {install_command})" + ) + self.gateway = gateway + self.dependency = dependency + self.env = env + self.install_command = install_command + + +class EngineVersionError(PowderLineError): + """The installed engine version is outside the gateway's supported range.""" + + def __init__(self, *, gateway: str, engine: str, installed: str, supported: str) -> None: + super().__init__( + f"the {gateway!r} gateway supports {engine} {supported}, but {installed} " + "is installed; install a supported version" + ) + self.gateway = gateway + self.engine = engine + self.installed = installed + self.supported = supported + + +class EngineExecutionError(PowderLineError): + """The engine ran and failed.""" + + +class StructuredWarning(TypedDict): + """One entry of the ``warnings`` list in results and ``validate()`` output. + + ``field_path`` is the dotted recipe path the warning refers to, or ``None`` + when it is not tied to one field. Plumbed into results per gateway in + re/04-06. + """ + + code: str + message: str + field_path: Optional[str] diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py new file mode 100644 index 0000000..5ae9aaf --- /dev/null +++ b/tests/test_exceptions.py @@ -0,0 +1,77 @@ +"""Unit tests for powderline.exceptions (hierarchy, messages, warning shape). + +Pure tests: no engine, no network. +""" +import pytest +from pydantic import BaseModel, ValidationError + +from powderline.exceptions import ( + EngineExecutionError, + EngineNotAvailableError, + EngineVersionError, + GatewayNotInstalledError, + PowderLineError, + RecipeValidationError, + StructuredWarning, +) + + +@pytest.mark.parametrize("cls", [ + RecipeValidationError, GatewayNotInstalledError, EngineNotAvailableError, + EngineVersionError, EngineExecutionError, +]) +def test_all_derive_from_powderline_error(cls): + assert issubclass(cls, PowderLineError) + + +def test_engine_not_available_is_an_import_error(): + # A52: callers that caught the underlying ImportError keep working. + err = EngineNotAvailableError(gateway="easydiffraction", dependency="easydiffraction", + env="easydiff", install_command="pixi install -e easydiff") + with pytest.raises(ImportError): + raise err + + +def test_engine_not_available_message_names_gateway_env_and_command(): + err = EngineNotAvailableError(gateway="easydiffraction", dependency="easydiffraction", + env="easydiff", install_command="pixi install -e easydiff") + msg = str(err) + assert "'easydiffraction' gateway" in msg + assert "'easydiff' pixi environment" in msg + assert "pixi install -e easydiff" in msg + assert (err.gateway, err.env) == ("easydiffraction", "easydiff") + + +def test_engine_version_error_message(): + err = EngineVersionError(gateway="gsasii", engine="GSAS-II", installed="5.8.0", + supported="==5.7.9") + assert str(err) == ("the 'gsasii' gateway supports GSAS-II ==5.7.9, but 5.8.0 is " + "installed; install a supported version") + assert (err.installed, err.supported) == ("5.8.0", "==5.7.9") + + +def test_gateway_not_installed_keeps_name(): + err = GatewayNotInstalledError("no gateway 'foo'", gateway="foo") + assert err.gateway == "foo" + assert str(err) == "no gateway 'foo'" + + +class _M(BaseModel): + x: int + + +def test_recipe_validation_error_from_pydantic(): + with pytest.raises(ValidationError) as info: + _M.model_validate({"x": "not-an-int"}) + err = RecipeValidationError.from_pydantic(info.value, schema_name="GSASII_Rietveld") + assert isinstance(err, ValueError) + assert err.schema_name == "GSASII_Rietveld" + assert err.errors and err.errors[0]["loc"] == ("x",) + assert err.__cause__ is info.value + assert str(err).startswith("GSASII_Rietveld recipe failed validation:") + + +def test_structured_warning_shape(): + assert set(StructuredWarning.__annotations__) == {"code", "message", "field_path"} + w: StructuredWarning = {"code": "W001", "message": "m", "field_path": None} + assert w["field_path"] is None From b9767a7060482efe06c3916c7c17c3c8bf196eac Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:48:53 -0400 Subject: [PATCH 02/10] refactor(gsasii): move validate + validate_simulation_mode_parameters to an engine-free module (verbatim) Both functions read only the recipe (no GSAS-II references), but lived in kicker.py, which imports GSAS-II at module top. Moving them verbatim to gateways/gsasii/validation.py lets the gsasii gateway's schema layer validate without GSAS-II installed (A51). kicker re-exports both, so powderline.validate and every powderline.kicker.* import are unchanged. Co-Authored-By: Claude Opus 5.5 --- src/powderline/gateways/gsasii/validation.py | 252 +++++++++++++++++++ src/powderline/kicker.py | 243 +----------------- 2 files changed, 257 insertions(+), 238 deletions(-) create mode 100644 src/powderline/gateways/gsasii/validation.py diff --git a/src/powderline/gateways/gsasii/validation.py b/src/powderline/gateways/gsasii/validation.py new file mode 100644 index 0000000..dace7a4 --- /dev/null +++ b/src/powderline/gateways/gsasii/validation.py @@ -0,0 +1,252 @@ +"""Engine-free recipe validation for the GSAS-II gateway. + +``validate`` and ``validate_simulation_mode_parameters`` moved here verbatim from +``powderline.kicker`` (multi-engine refactor, re/02, A51) so the gateway's schema +layer can validate a recipe without GSAS-II installed. ``powderline.kicker`` +re-exports both, so existing imports keep working. + +This module must never import GSAS-II (enforced by an import-block test). +""" +from __future__ import annotations +from typing import List +from pydantic import ValidationError +from powderline.schema import RecipeModel +from powderline._status import CHECK, CROSS, WARN + + +def validate_simulation_mode_parameters(recipe: RecipeModel, verbose: bool = False) -> tuple[bool, List[str]]: + """ + Validate that simulation mode examples have all refinement parameters locked. + + Simulation mode (refinement_cycles=1 with all parameters fixed) must have deterministic + output. This function checks for any `refine_flag: true` in structural or background + parameters that would violate determinism. + + Args: + recipe: Validated RecipeModel instance (uses payload structure) + verbose: If True, print detailed parameter checks + + Returns: + (is_valid, warnings): is_valid is True if all parameters are properly locked, + warnings is a list of issues found (empty if valid). + + Examples: + >>> is_valid, warnings = validate_simulation_mode_parameters(recipe) + >>> if not is_valid: + ... for warning in warnings: + ... print(f"WARNING: {warning}") + """ + is_valid = True + warnings: List[str] = [] + + # Get refinement controls (in payload) + controls = recipe.payload.refinement_controls + + # Only check simulation mode (refinement_cycles == 1) + if controls.refinement_cycles != 1: + return True, [] # Not simulation mode, skip validation + + if verbose: + print("Validating simulation mode constraints...") + + # Check background parameters + if recipe.payload.background is not None: + if recipe.payload.background.chebyshev is not None: + if recipe.payload.background.chebyshev.refine_flag: + msg = "CRITICAL: Chebyshev background has refine_flag=true in simulation mode (refinement_cycles=1). " \ + "This violates determinism - all parameters must be locked. Set refine_flag=false." + warnings.append(msg) + is_valid = False + if verbose: + print(f" {CROSS} {msg}") + + if recipe.payload.background.single_peaks is not None: + # Check single peak parameters + param_lists = [ + (recipe.payload.background.single_peaks.positions, "single_peaks.positions"), + (recipe.payload.background.single_peaks.intensities, "single_peaks.intensities"), + (recipe.payload.background.single_peaks.pv_gaussian_sigma, "single_peaks.pv_gaussian_sigma"), + (recipe.payload.background.single_peaks.pv_lorentzian_gamma, "single_peaks.pv_lorentzian_gamma"), + ] + for param_list, param_name in param_lists: + if param_list is not None: + for i, param in enumerate(param_list): + if param[1]: # refine_flag is second element + msg = f"WARNING: {param_name}[{i}] has refine_flag=true in simulation mode. " \ + "Set refine_flag=false for deterministic output." + warnings.append(msg) + is_valid = False + if verbose: + print(f" {WARN} {msg}") + + # Check instrument parameterization + if recipe.payload.instrument.parameterization is not None: + param_dict = recipe.payload.instrument.parameterization.model_dump(mode='json') + + # Check wavelength + if param_dict.get('wavelength') and param_dict['wavelength'][1]: + msg = "WARNING: instrument.parameterization.wavelength has refine_flag=true in simulation mode" + warnings.append(msg) + is_valid = False + if verbose: + print(f" {WARN} {msg}") + + # Check broadening parameters + if param_dict.get('broadening'): + for param_key, param_value in param_dict['broadening'].items(): + if param_value and param_value[1]: # refine_flag + msg = f"WARNING: instrument.parameterization.broadening.{param_key} has refine_flag=true in simulation mode" + warnings.append(msg) + is_valid = False + if verbose: + print(f" {WARN} {msg}") + + # Check corrections + if param_dict.get('corrections'): + for param_key, param_value in param_dict['corrections'].items(): + if param_value and param_value[1]: # refine_flag + msg = f"WARNING: instrument.parameterization.corrections.{param_key} has refine_flag=true in simulation mode" + warnings.append(msg) + is_valid = False + if verbose: + print(f" {WARN} {msg}") + + # Check phase-level parameters (only if phases exist) + if recipe.payload.phases is not None: + phases_dict = recipe.payload.model_dump(mode='json')['phases'] + for phase_name, phase_info in phases_dict.items(): + param_dict = phase_info.get('parameterization', {}) + + # Check scale + if param_dict.get('scale') and param_dict['scale'][1]: + msg = f"WARNING: phases.{phase_name}.parameterization.scale has refine_flag=true in simulation mode" + warnings.append(msg) + is_valid = False + if verbose: + print(f" {WARN} {msg}") + + # Check unit cell + unit_cell = param_dict.get('unit_cell', {}) + if unit_cell: + for cell_param, param_value in unit_cell.items(): + if param_value and param_value[1]: # refine_flag + msg = f"WARNING: phases.{phase_name}.parameterization.unit_cell.{cell_param} has refine_flag=true in simulation mode" + warnings.append(msg) + is_valid = False + if verbose: + print(f" {WARN} {msg}") + + # Check peak broadening + peak_broadening = param_dict.get('peak_broadening', {}) + if peak_broadening: + # Check size broadening + if peak_broadening.get('size_broadening'): + for param_key, param_value in peak_broadening['size_broadening'].items(): + if param_key == 'model': # Skip model field (string literal, not [value, refine_flag]) + continue + if param_value and param_value[1]: # refine_flag + msg = f"WARNING: phases.{phase_name}.parameterization.peak_broadening.size_broadening.{param_key} has refine_flag=true" + warnings.append(msg) + is_valid = False + if verbose: + print(f" {WARN} {msg}") + + # Check strain broadening + if peak_broadening.get('strain_broadening'): + for param_key, param_value in peak_broadening['strain_broadening'].items(): + if param_key == 'model': # Skip model field (string literal, not [value, refine_flag]) + continue + if param_value and param_value[1]: # refine_flag + msg = f"WARNING: phases.{phase_name}.parameterization.peak_broadening.strain_broadening.{param_key} has refine_flag=true" + warnings.append(msg) + is_valid = False + if verbose: + print(f" {WARN} {msg}") + + # Check atom parameters (using same logic as has_active_refinement_parameter for consistency) + atoms = param_dict.get('atoms', {}) + if atoms: + for atom_label, atom_params in atoms.items(): + for param_key, param_value in atom_params.items(): + # Skip ADP field (it's a string, not a RefinementParameter) + if param_key == 'ADP': + continue + + # Handle Uaniso nested dict + if param_key == 'Uaniso' and isinstance(param_value, dict): + for uaniso_key, uaniso_value in param_value.items(): + if isinstance(uaniso_value, list) and len(uaniso_value) >= 2 and uaniso_value[1]: + msg = f"WARNING: phases.{phase_name}.parameterization.atoms.{atom_label}.Uaniso.{uaniso_key} has refine_flag=true in simulation mode" + warnings.append(msg) + is_valid = False + if verbose: + print(f" {WARN} {msg}") + + # Check regular RefinementParameter lists (use elif to avoid checking lists after Uaniso) + elif isinstance(param_value, list) and len(param_value) >= 2 and param_value[1]: + msg = f"WARNING: phases.{phase_name}.parameterization.atoms.{atom_label}.{param_key} has refine_flag=true in simulation mode" + warnings.append(msg) + is_valid = False + if verbose: + print(f" {WARN} {msg}") + + if verbose: + if is_valid: + print(f"{CHECK} All parameters properly locked for simulation mode determinism") + + return is_valid, warnings + + +def validate(recipe: RecipeModel | dict, verbose: bool = False) -> RecipeModel: + """Validate a PowderLine recipe without running a refinement. + + Performs full Pydantic schema validation and simulation-mode consistency + checks. Raises ``pydantic.ValidationError`` immediately on any schema + violation so callers receive structured, actionable error information. + + This function is useful when batch-creating recipes and you want to + confirm each one is valid before committing to a refinement run. + + Note: The ``is_template_file()`` guard is **not** applied here. That + check is CLI-only (guards against accidentally running bare template + files from disk). When calling ``validate()`` programmatically the + caller is responsible for ensuring the recipe contains real data. + + Args: + recipe: A fully-populated ``RecipeModel`` instance or equivalent + dict. XRD data must already be present in + ``recipe.payload.xrd_data``. + verbose: If True, print simulation-mode warnings to stdout. + + Returns: + Validated ``RecipeModel`` instance. + + Raises: + pydantic.ValidationError: If the recipe fails schema validation. + ValueError: If simulation-mode constraints are violated (all + ``refine_flag`` fields must be ``false`` when + ``refinement_cycles == 1``). + + Example:: + + import json + import powderline + + recipe_dict = json.load(open("my_recipe.json")) + recipe_model = powderline.validate(recipe_dict) + print(f"Schema: {recipe_model.schema_name}, phases: {len(recipe_model.payload.phases or [])}") + """ + from pydantic import ValidationError # already imported at module level, re-stated for clarity + + if not isinstance(recipe, RecipeModel): + recipe = RecipeModel.model_validate(recipe) # raises ValidationError on failure + + is_sim_valid, sim_warnings = validate_simulation_mode_parameters(recipe, verbose=verbose) + if not is_sim_valid: + raise ValueError( + "Simulation mode validation failed. All refine_flag fields must be false " + f"when refinement_cycles == 1.\nConstraints violated:\n" + + "\n".join(f" - {w}" for w in sim_warnings) + ) + + return recipe diff --git a/src/powderline/kicker.py b/src/powderline/kicker.py index 36f3ddc..c723e87 100644 --- a/src/powderline/kicker.py +++ b/src/powderline/kicker.py @@ -97,6 +97,11 @@ _extract_refined_parameters, ) +from powderline.gateways.gsasii.validation import ( # noqa: F401 + validate_simulation_mode_parameters, + validate, +) + def load_recipe_asset(recipe_path: Path) -> dict: """Load a recipe asset from .json, .yaml/.yml, or .txt (YAML).""" @@ -164,248 +169,10 @@ def is_template_file(recipe_dict: dict, input_path: Path) -> tuple[bool, str | N return False, None -def validate_simulation_mode_parameters(recipe: RecipeModel, verbose: bool = False) -> tuple[bool, List[str]]: - """ - Validate that simulation mode examples have all refinement parameters locked. - - Simulation mode (refinement_cycles=1 with all parameters fixed) must have deterministic - output. This function checks for any `refine_flag: true` in structural or background - parameters that would violate determinism. - - Args: - recipe: Validated RecipeModel instance (uses payload structure) - verbose: If True, print detailed parameter checks - - Returns: - (is_valid, warnings): is_valid is True if all parameters are properly locked, - warnings is a list of issues found (empty if valid). - - Examples: - >>> is_valid, warnings = validate_simulation_mode_parameters(recipe) - >>> if not is_valid: - ... for warning in warnings: - ... print(f"WARNING: {warning}") - """ - is_valid = True - warnings: List[str] = [] - - # Get refinement controls (in payload) - controls = recipe.payload.refinement_controls - - # Only check simulation mode (refinement_cycles == 1) - if controls.refinement_cycles != 1: - return True, [] # Not simulation mode, skip validation - - if verbose: - print("Validating simulation mode constraints...") - - # Check background parameters - if recipe.payload.background is not None: - if recipe.payload.background.chebyshev is not None: - if recipe.payload.background.chebyshev.refine_flag: - msg = "CRITICAL: Chebyshev background has refine_flag=true in simulation mode (refinement_cycles=1). " \ - "This violates determinism - all parameters must be locked. Set refine_flag=false." - warnings.append(msg) - is_valid = False - if verbose: - print(f" {CROSS} {msg}") - - if recipe.payload.background.single_peaks is not None: - # Check single peak parameters - param_lists = [ - (recipe.payload.background.single_peaks.positions, "single_peaks.positions"), - (recipe.payload.background.single_peaks.intensities, "single_peaks.intensities"), - (recipe.payload.background.single_peaks.pv_gaussian_sigma, "single_peaks.pv_gaussian_sigma"), - (recipe.payload.background.single_peaks.pv_lorentzian_gamma, "single_peaks.pv_lorentzian_gamma"), - ] - for param_list, param_name in param_lists: - if param_list is not None: - for i, param in enumerate(param_list): - if param[1]: # refine_flag is second element - msg = f"WARNING: {param_name}[{i}] has refine_flag=true in simulation mode. " \ - "Set refine_flag=false for deterministic output." - warnings.append(msg) - is_valid = False - if verbose: - print(f" {WARN} {msg}") - - # Check instrument parameterization - if recipe.payload.instrument.parameterization is not None: - param_dict = recipe.payload.instrument.parameterization.model_dump(mode='json') - - # Check wavelength - if param_dict.get('wavelength') and param_dict['wavelength'][1]: - msg = "WARNING: instrument.parameterization.wavelength has refine_flag=true in simulation mode" - warnings.append(msg) - is_valid = False - if verbose: - print(f" {WARN} {msg}") - - # Check broadening parameters - if param_dict.get('broadening'): - for param_key, param_value in param_dict['broadening'].items(): - if param_value and param_value[1]: # refine_flag - msg = f"WARNING: instrument.parameterization.broadening.{param_key} has refine_flag=true in simulation mode" - warnings.append(msg) - is_valid = False - if verbose: - print(f" {WARN} {msg}") - - # Check corrections - if param_dict.get('corrections'): - for param_key, param_value in param_dict['corrections'].items(): - if param_value and param_value[1]: # refine_flag - msg = f"WARNING: instrument.parameterization.corrections.{param_key} has refine_flag=true in simulation mode" - warnings.append(msg) - is_valid = False - if verbose: - print(f" {WARN} {msg}") - - # Check phase-level parameters (only if phases exist) - if recipe.payload.phases is not None: - phases_dict = recipe.payload.model_dump(mode='json')['phases'] - for phase_name, phase_info in phases_dict.items(): - param_dict = phase_info.get('parameterization', {}) - - # Check scale - if param_dict.get('scale') and param_dict['scale'][1]: - msg = f"WARNING: phases.{phase_name}.parameterization.scale has refine_flag=true in simulation mode" - warnings.append(msg) - is_valid = False - if verbose: - print(f" {WARN} {msg}") - - # Check unit cell - unit_cell = param_dict.get('unit_cell', {}) - if unit_cell: - for cell_param, param_value in unit_cell.items(): - if param_value and param_value[1]: # refine_flag - msg = f"WARNING: phases.{phase_name}.parameterization.unit_cell.{cell_param} has refine_flag=true in simulation mode" - warnings.append(msg) - is_valid = False - if verbose: - print(f" {WARN} {msg}") - - # Check peak broadening - peak_broadening = param_dict.get('peak_broadening', {}) - if peak_broadening: - # Check size broadening - if peak_broadening.get('size_broadening'): - for param_key, param_value in peak_broadening['size_broadening'].items(): - if param_key == 'model': # Skip model field (string literal, not [value, refine_flag]) - continue - if param_value and param_value[1]: # refine_flag - msg = f"WARNING: phases.{phase_name}.parameterization.peak_broadening.size_broadening.{param_key} has refine_flag=true" - warnings.append(msg) - is_valid = False - if verbose: - print(f" {WARN} {msg}") - - # Check strain broadening - if peak_broadening.get('strain_broadening'): - for param_key, param_value in peak_broadening['strain_broadening'].items(): - if param_key == 'model': # Skip model field (string literal, not [value, refine_flag]) - continue - if param_value and param_value[1]: # refine_flag - msg = f"WARNING: phases.{phase_name}.parameterization.peak_broadening.strain_broadening.{param_key} has refine_flag=true" - warnings.append(msg) - is_valid = False - if verbose: - print(f" {WARN} {msg}") - - # Check atom parameters (using same logic as has_active_refinement_parameter for consistency) - atoms = param_dict.get('atoms', {}) - if atoms: - for atom_label, atom_params in atoms.items(): - for param_key, param_value in atom_params.items(): - # Skip ADP field (it's a string, not a RefinementParameter) - if param_key == 'ADP': - continue - - # Handle Uaniso nested dict - if param_key == 'Uaniso' and isinstance(param_value, dict): - for uaniso_key, uaniso_value in param_value.items(): - if isinstance(uaniso_value, list) and len(uaniso_value) >= 2 and uaniso_value[1]: - msg = f"WARNING: phases.{phase_name}.parameterization.atoms.{atom_label}.Uaniso.{uaniso_key} has refine_flag=true in simulation mode" - warnings.append(msg) - is_valid = False - if verbose: - print(f" {WARN} {msg}") - - # Check regular RefinementParameter lists (use elif to avoid checking lists after Uaniso) - elif isinstance(param_value, list) and len(param_value) >= 2 and param_value[1]: - msg = f"WARNING: phases.{phase_name}.parameterization.atoms.{atom_label}.{param_key} has refine_flag=true in simulation mode" - warnings.append(msg) - is_valid = False - if verbose: - print(f" {WARN} {msg}") - - if verbose: - if is_valid: - print(f"{CHECK} All parameters properly locked for simulation mode determinism") - - return is_valid, warnings - - ######################################## # Public programmatic API ######################################## -def validate(recipe: RecipeModel | dict, verbose: bool = False) -> RecipeModel: - """Validate a PowderLine recipe without running a refinement. - - Performs full Pydantic schema validation and simulation-mode consistency - checks. Raises ``pydantic.ValidationError`` immediately on any schema - violation so callers receive structured, actionable error information. - - This function is useful when batch-creating recipes and you want to - confirm each one is valid before committing to a refinement run. - - Note: The ``is_template_file()`` guard is **not** applied here. That - check is CLI-only (guards against accidentally running bare template - files from disk). When calling ``validate()`` programmatically the - caller is responsible for ensuring the recipe contains real data. - - Args: - recipe: A fully-populated ``RecipeModel`` instance or equivalent - dict. XRD data must already be present in - ``recipe.payload.xrd_data``. - verbose: If True, print simulation-mode warnings to stdout. - - Returns: - Validated ``RecipeModel`` instance. - - Raises: - pydantic.ValidationError: If the recipe fails schema validation. - ValueError: If simulation-mode constraints are violated (all - ``refine_flag`` fields must be ``false`` when - ``refinement_cycles == 1``). - - Example:: - - import json - import powderline - - recipe_dict = json.load(open("my_recipe.json")) - recipe_model = powderline.validate(recipe_dict) - print(f"Schema: {recipe_model.schema_name}, phases: {len(recipe_model.payload.phases or [])}") - """ - from pydantic import ValidationError # already imported at module level, re-stated for clarity - - if not isinstance(recipe, RecipeModel): - recipe = RecipeModel.model_validate(recipe) # raises ValidationError on failure - - is_sim_valid, sim_warnings = validate_simulation_mode_parameters(recipe, verbose=verbose) - if not is_sim_valid: - raise ValueError( - "Simulation mode validation failed. All refine_flag fields must be false " - f"when refinement_cycles == 1.\nConstraints violated:\n" - + "\n".join(f" - {w}" for w in sim_warnings) - ) - - return recipe - - def run( recipe: RecipeModel | dict, output_dir: Path, From b3eec021920d0f622a6c068a70793e70ab8f6c6c Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:56:07 -0400 Subject: [PATCH 03/10] refactor(core): promote symmetry out of topas/ into powderline.symmetry Moves src/powderline/topas/symmetry.py to src/powderline/symmetry.py, ending the easydiff -> topas cross-adapter import. Importers updated (topas writer, easydiff policy + builder, test_topas_symmetry); no shim. One deviation from a verbatim move (A56): the module's relative 'from .errors import TopasTranslationError' cannot survive the move, and an absolute import at module top is circular (topas/__init__ -> writer -> symmetry). The import now happens inside the two raising functions, so the raised type is unchanged. Interim: re/03 makes core symmetry raise a core error that each gateway translates. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- src/powderline/easydiff/builder.py | 2 +- src/powderline/easydiff/policy.py | 2 +- src/powderline/{topas => }/symmetry.py | 9 ++++++++- src/powderline/topas/writer.py | 2 +- tests/test_topas_symmetry.py | 4 ++-- 6 files changed, 14 insertions(+), 7 deletions(-) rename src/powderline/{topas => }/symmetry.py (96%) diff --git a/CLAUDE.md b/CLAUDE.md index 582b4bf..8e5b8e2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,7 +70,7 @@ Python ≥3.12): - `policy.py` — the pre-flight "honesty rule": fixed-unmappable recipe features are dropped with a recorded warning; anything *flagged for refinement* that can't be represented raises `errors.EasyDiffractionTranslationError`. Also - validates space groups via `topas.symmetry.cell_constraints`. Powers + validates space groups via `powderline.symmetry.cell_constraints`. Powers `validate_only` with **no easydiffraction import**. - `builder.py` — recipe dict → easydiffraction `Project` + a manifest mapping every freed parameter back to GSAS-II naming/units (`scale_to_recipe`); cell diff --git a/src/powderline/easydiff/builder.py b/src/powderline/easydiff/builder.py index a8fcec9..6cd44c9 100644 --- a/src/powderline/easydiff/builder.py +++ b/src/powderline/easydiff/builder.py @@ -23,7 +23,7 @@ from powderline.easydiff.errors import EasyDiffractionTranslationError from powderline.easydiff.policy import check_unsupported, param_flag, param_value from powderline.topas.errors import TopasTranslationError -from powderline.topas.symmetry import cell_constraints +from powderline.symmetry import cell_constraints @dataclass diff --git a/src/powderline/easydiff/policy.py b/src/powderline/easydiff/policy.py index b140f1e..3cf9a6f 100644 --- a/src/powderline/easydiff/policy.py +++ b/src/powderline/easydiff/policy.py @@ -35,7 +35,7 @@ def _iparm_current(iparm1: dict, key: str, default=0.0): def check_unsupported(recipe: dict) -> list[str]: - from powderline.topas.symmetry import cell_constraints + from powderline.symmetry import cell_constraints from powderline.topas.errors import TopasTranslationError warnings: list[str] = [] diff --git a/src/powderline/topas/symmetry.py b/src/powderline/symmetry.py similarity index 96% rename from src/powderline/topas/symmetry.py rename to src/powderline/symmetry.py index f75d405..e38aedf 100644 --- a/src/powderline/topas/symmetry.py +++ b/src/powderline/symmetry.py @@ -20,7 +20,6 @@ import gemmi import numpy as np -from .errors import TopasTranslationError # Fractional-coordinate tolerances (plan §5). _POS_TOL = 1e-5 # position / orbit equality mod 1 @@ -97,6 +96,10 @@ def resolve_space_group(space_group: str) -> gemmi.SpaceGroup: passed without an explicit ``:1``/``:2`` selector also error rather than guess an origin (plan §5(6); demo groups are single-origin). """ + # Lazy: importing powderline.topas at module top is circular (topas/__init__ + # -> writer -> symmetry). Interim until re/03 raises a core error here (A56). + from powderline.topas.errors import TopasTranslationError + raw = str(space_group) sg = gemmi.find_spacegroup_by_name(raw) if sg is None: @@ -138,6 +141,10 @@ def _monoclinic_unique_axis(ops: list[tuple[np.ndarray, np.ndarray]]) -> str: The single proper 2-fold (det +1, trace -1) fixes the unique axis; its invariant direction maps to the dominant cell axis a/b/c. """ + # Lazy: importing powderline.topas at module top is circular (topas/__init__ + # -> writer -> symmetry). Interim until re/03 raises a core error here (A56). + from powderline.topas.errors import TopasTranslationError + for R, _t in ops: if abs(np.linalg.det(R) - 1.0) < _MAT_TOL and abs(np.trace(R) + 1.0) < _MAT_TOL: evals, evecs = np.linalg.eig(R) diff --git a/src/powderline/topas/writer.py b/src/powderline/topas/writer.py index c14cea1..e3e9978 100644 --- a/src/powderline/topas/writer.py +++ b/src/powderline/topas/writer.py @@ -24,7 +24,7 @@ from . import conversions as cv from .errors import TopasTranslationError -from .symmetry import adp_dof, cell_constraints, site_dof +from powderline.symmetry import adp_dof, cell_constraints, site_dof SCHEMA_RIETVELD = "GSASII_Rietveld" SCHEMA_SPF = "GSASII_SPF" diff --git a/tests/test_topas_symmetry.py b/tests/test_topas_symmetry.py index f45bc34..27295c1 100644 --- a/tests/test_topas_symmetry.py +++ b/tests/test_topas_symmetry.py @@ -1,10 +1,10 @@ -"""Tests for powderline.topas.symmetry (plan §5 targets + error cases).""" +"""Tests for powderline.symmetry (plan §5 targets + error cases).""" from __future__ import annotations import pytest -from powderline.topas import symmetry as sym +from powderline import symmetry as sym from powderline.topas.errors import TopasTranslationError From a42a8a7bb98d0e245287b64285a56e3ec9f2c545 Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:56:16 -0400 Subject: [PATCH 04/10] feat(core): add fitstats - uniform Rwp, Rexp, GoF, reduced chi-squared Pure-numpy standard fit statistics (A23), computed once in core. R factors in percent, GoF/chi2_red dimensionless (A55). N = points inside the fit-range mask with w > 0 (A55; evidence in the dossier's tasks/fitstats-n-convention.md); P is the engine-reported varied-parameter count, supplied by each gateway (A41/A45). Created + unit-tested only; gateways adopt it in re/04-06. The LaB6 characterization test documents how it differs from GSAS-II's own numbers: Rwp/GoF agree; GSAS-II's wRmin omits P; N differs by one edge point. Co-Authored-By: Claude Opus 5.5 --- src/powderline/fitstats.py | 99 +++++++++++++++++++++++++++++++ tests/test_fitstats.py | 117 +++++++++++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+) create mode 100644 src/powderline/fitstats.py create mode 100644 tests/test_fitstats.py diff --git a/src/powderline/fitstats.py b/src/powderline/fitstats.py new file mode 100644 index 0000000..7b4938b --- /dev/null +++ b/src/powderline/fitstats.py @@ -0,0 +1,99 @@ +"""Uniform fit statistics computed once in core (master plan A23, A41, A45). + +Every gateway reports the same standard keys (Rwp, Rexp, GoF, reduced χ²) +computed here from the observed/calculated profile, so the numbers mean the +same thing whichever engine ran. Engine-reported values are preserved +separately (``engine_details``); how and why they differ is documented, not +hidden. + +Formulas (the user-docs formulas), with sums over the included points ``i``:: + + Rwp = 100 · sqrt( Σ w_i (yobs_i − ycalc_i)² / Σ w_i yobs_i² ) [%] + Rexp = 100 · sqrt( (N − P) / Σ w_i yobs_i² ) [%] + GoF = Rwp / Rexp = sqrt( Σ w_i (yobs_i − ycalc_i)² / (N − P) ) + χ²_red = Σ w_i (yobs_i − ycalc_i)² / (N − P) (= GoF²) + +- **Included points / N**: inside the fit-range ``mask`` *and* with + ``w_i > 0``. Excluded and out-of-range points are left out of N and the sums + (the convention across GSAS-II, TOPAS and FullProf). Zero-weight points are + also left out: they add no equation to the weighted least-squares problem, so + they add no degree of freedom. GSAS-II counts in-range zero-weight points in + its ``Nobs``; see ``tasks/fitstats-n-convention.md`` in the dossier. +- **P** (``n_params``): the engine's own count of independently varied + parameters after engine-applied constraints, read from the engine **after the + run** (A41, A45) — never derived from recipe refine flags. Each gateway + supplies it. +- R factors are in percent; GoF and χ²_red are dimensionless. +- Undefined values are ``nan``: Rexp, GoF and χ²_red when ``N − P <= 0``; + Rwp and Rexp when ``Σ w yobs² == 0``. + +Pure numpy; imports no engine. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Optional + +import numpy as np + + +@dataclass(frozen=True) +class FitStatistics: + """Standard fit statistics; see the module docstring for the formulas.""" + + rwp: float + rexp: float + gof: float + chi2_red: float + n_points: int + n_params: int + + +def compute_fit_statistics(yobs, ycalc, weights, n_params: int, + mask: Optional[np.ndarray] = None) -> FitStatistics: + """Compute Rwp, Rexp, GoF and reduced χ² over the included points. + + Args: + yobs, ycalc, weights: 1-D arrays of equal length (weights ``>= 0``). + n_params: P, the engine-reported number of varied parameters. + mask: optional boolean array, True = inside the fit range; default all + points. + + Raises: + ValueError: on mismatched shapes, negative weights, a negative + ``n_params``, or non-finite values among the included points. + """ + yobs = np.asarray(yobs, dtype=float) + ycalc = np.asarray(ycalc, dtype=float) + weights = np.asarray(weights, dtype=float) + if yobs.ndim != 1 or yobs.shape != ycalc.shape or yobs.shape != weights.shape: + raise ValueError("yobs, ycalc and weights must be 1-D arrays of equal length") + if mask is None: + mask = np.ones(yobs.shape, dtype=bool) + else: + mask = np.asarray(mask, dtype=bool) + if mask.shape != yobs.shape: + raise ValueError("mask must have the same length as yobs") + if int(n_params) != n_params or n_params < 0: + raise ValueError(f"n_params must be a non-negative integer, got {n_params!r}") + n_params = int(n_params) + if np.any(weights[mask] < 0): + raise ValueError("weights must be >= 0") + + included = mask & (weights > 0) + yo, yc, w = yobs[included], ycalc[included], weights[included] + if not (np.all(np.isfinite(yo)) and np.all(np.isfinite(yc)) and np.all(np.isfinite(w))): + raise ValueError("yobs, ycalc and weights must be finite inside the fit range") + + n_points = int(included.sum()) + sum_wyo2 = float(np.sum(w * yo**2)) + sum_wr2 = float(np.sum(w * (yo - yc) ** 2)) + dof = n_points - n_params + + rwp = 100.0 * np.sqrt(sum_wr2 / sum_wyo2) if sum_wyo2 > 0 else float("nan") + rexp = 100.0 * np.sqrt(dof / sum_wyo2) if (sum_wyo2 > 0 and dof > 0) else float("nan") + chi2_red = sum_wr2 / dof if dof > 0 else float("nan") + gof = float(np.sqrt(chi2_red)) if dof > 0 else float("nan") + return FitStatistics(rwp=float(rwp), rexp=float(rexp), gof=gof, chi2_red=float(chi2_red), + n_points=n_points, n_params=n_params) diff --git a/tests/test_fitstats.py b/tests/test_fitstats.py new file mode 100644 index 0000000..c2bc674 --- /dev/null +++ b/tests/test_fitstats.py @@ -0,0 +1,117 @@ +"""Unit tests for powderline.fitstats (uniform Rwp / Rexp / GoF / reduced χ²). + +Pure tests: no engine. Synthetic cases are hand-computed; the LaB6 case +characterizes (does not assert equality with) GSAS-II's own reported values. +""" +import json +import math +from pathlib import Path + +import numpy as np +import pandas as pd +import pytest + +from powderline.fitstats import FitStatistics, compute_fit_statistics + +EXAMPLES = Path(__file__).resolve().parent.parent / "examples" + + +def test_hand_computed_case(): + # yobs=[10,20,30,40], ycalc=[11,19,30,42], w=[1,0.5,0.25,0.1], P=1 + # Σw·yobs² = 100 + 200 + 225 + 160 = 685 + # Σw·(yobs−ycalc)² = 1 + 0.5 + 0 + 0.4 = 1.9 + # N = 4, N−P = 3 + s = compute_fit_statistics([10, 20, 30, 40], [11, 19, 30, 42], [1, 0.5, 0.25, 0.1], 1) + assert s.rwp == pytest.approx(100 * math.sqrt(1.9 / 685)) + assert s.rexp == pytest.approx(100 * math.sqrt(3 / 685)) + assert s.chi2_red == pytest.approx(1.9 / 3) + assert s.gof == pytest.approx(math.sqrt(1.9 / 3)) + assert s.gof == pytest.approx(s.rwp / s.rexp) + assert (s.n_points, s.n_params) == (4, 1) + + +def test_perfect_fit_is_zero(): + s = compute_fit_statistics([1.0, 2.0, 3.0], [1.0, 2.0, 3.0], [1, 1, 1], 0) + assert s.rwp == 0.0 and s.gof == 0.0 and s.chi2_red == 0.0 + assert s.rexp == pytest.approx(100 * math.sqrt(3 / 14)) + + +def test_mask_and_zero_weights_are_excluded_from_n_and_sums(): + base = compute_fit_statistics([10, 20], [11, 19], [1, 0.5], 1) + padded = compute_fit_statistics( + [10, 20, 999, 5], [11, 19, 0, 7], [1, 0.5, 1, 0], 1, + mask=[True, True, False, True], # point 2 out of range; point 3 has w=0 + ) + assert padded == base + assert padded.n_points == 2 + + +def test_undefined_values_are_nan(): + s = compute_fit_statistics([1.0, 2.0], [1.5, 2.5], [1, 1], 2) # N − P = 0 + assert s.rwp == pytest.approx(100 * math.sqrt(0.5 / 5)) + assert math.isnan(s.rexp) and math.isnan(s.gof) and math.isnan(s.chi2_red) + z = compute_fit_statistics([0.0, 0.0], [1.0, 1.0], [1, 1], 0) # Σw·yobs² = 0 + assert math.isnan(z.rwp) and math.isnan(z.rexp) + assert z.chi2_red == pytest.approx(1.0) + + +@pytest.mark.parametrize("kwargs, match", [ + (dict(yobs=[1, 2], ycalc=[1], weights=[1, 1], n_params=0), "equal length"), + (dict(yobs=[1, 2], ycalc=[1, 2], weights=[1, -1], n_params=0), ">= 0"), + (dict(yobs=[1, 2], ycalc=[1, 2], weights=[1, 1], n_params=-1), "non-negative integer"), + (dict(yobs=[1, 2], ycalc=[1, 2], weights=[1, 1], n_params=1.5), "non-negative integer"), + (dict(yobs=[1, np.nan], ycalc=[1, 2], weights=[1, 1], n_params=0), "finite"), + (dict(yobs=[1, 2], ycalc=[1, 2], weights=[1, 1], n_params=0, mask=[True]), "mask"), +]) +def test_invalid_input_raises(kwargs, match): + with pytest.raises(ValueError, match=match): + compute_fit_statistics(**kwargs) + + +def test_non_finite_outside_fit_range_is_ignored(): + s = compute_fit_statistics([1.0, np.nan], [1.0, 0.0], [1, 1], 0, mask=[True, False]) + assert s.n_points == 1 + + +def test_result_is_frozen(): + s = compute_fit_statistics([1.0], [1.0], [1.0], 0) + assert isinstance(s, FitStatistics) + with pytest.raises(AttributeError): + s.rwp = 1.0 + + +def test_lab6_characterization_vs_gsasii(): + """Characterize (not assert equality with) GSAS-II on the committed LaB6 fit. + + Inputs: ``examples/example_LaB6/output/fit_profile.txt`` (4096 points, values + rounded to 8 decimals), fit range [1, 15]° 2θ from the recipe, P = 17 from + ``dummy.lst`` ("No. of parameters: 17"). + + GSAS-II (``dummy.lst``): wR = 6.53 %, wRmin = 12.48 %, GOF = 0.52, + chi**2 = 1030.77, 3768 observations. + PowderLine fitstats: Rwp = 6.527 %, Rexp = 12.450 %, GoF = 0.524, + N = 3767. + + How and why they differ (A23): + - Rwp and GoF agree to the reported precision. + - N: GSAS-II slices the fit range by index (``x[xB:xF]``) and counts one + more point than the inclusive 2θ mask [1, 15] used here (3768 vs 3767). + - Rexp vs GSAS-II ``wRmin``: GSAS-II's ``wRmin`` = sqrt(N / Σw·yobs²), i.e. + it does **not** subtract P; fitstats uses the standard sqrt((N − P) / + Σw·yobs²). sqrt(3768/3751) accounts for the 12.48 vs 12.45 gap. + - GSAS-II's ``chi**2`` is the un-reduced Σw·(yobs − ycalc)²; χ²_red here is + that sum divided by (N − P). + """ + recipe = json.loads((EXAMPLES / "example_LaB6" / "input.json").read_text(encoding="utf-8")) + lo, hi = recipe["payload"]["fit_range"] + prof = pd.read_csv(EXAMPLES / "example_LaB6" / "output" / "fit_profile.txt", sep="\t") + tth = prof["two_theta"].to_numpy() + s = compute_fit_statistics(prof["y_obs"], prof["y_calc"], prof["y_weights"], 17, + mask=(tth >= lo) & (tth <= hi)) + assert s.n_points == 3767 + assert s.rwp == pytest.approx(6.53, abs=0.005) + assert s.gof == pytest.approx(0.52, abs=0.005) + assert s.rexp == pytest.approx(12.45, abs=0.005) + # GSAS-II's wRmin omits P: reproduce it from the same sums to prove the claim. + gsas_wrmin = s.rexp * math.sqrt(3768 / (3767 - 17)) + assert gsas_wrmin == pytest.approx(12.48, abs=0.005) From 9c241093d347f4438ffa02a85b5d3d05c12690f9 Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:56:16 -0400 Subject: [PATCH 05/10] feat(core): add reports - shared writers for the standardized report files Engine-free writers for refined_parameters.csv, _unit_cell_report.csv, fit_profile.txt and _peak_list_report.csv: they own file naming, column order, header, delimiter and quoting (csv QUOTE_MINIMAL, like pandas), UTF-8 + LF; number formatting stays with the caller. Created + unit-tested only (byte-identical round-trip of all 37 committed GSAS-II references); gateways adopt them in re/04-06 with byte-identical checks. Co-Authored-By: Claude Opus 5.5 --- src/powderline/reports.py | 196 +++++++++++++++++++ tests/test_reports.py | 399 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 595 insertions(+) create mode 100644 src/powderline/reports.py create mode 100644 tests/test_reports.py diff --git a/src/powderline/reports.py b/src/powderline/reports.py new file mode 100644 index 0000000..2126c41 --- /dev/null +++ b/src/powderline/reports.py @@ -0,0 +1,196 @@ +"""Shared writers for PowderLine's standardized report files. + +This module provides engine-free writers for the standardized report files +emitted by all PowderLine engines: ``refined_parameters.csv``, +``_unit_cell_report.csv``, ``fit_profile.txt``, and +``_peak_list_report.csv``. + +The writers own file naming, column order, header, delimiter, and quoting +(csv QUOTE_MINIMAL, like pandas). All files are written with UTF-8 encoding +and LF line endings (``encoding="utf-8", newline=""`` on open with csv +``lineterminator="\\n"``), ensuring deterministic output on all platforms. + +**Number formatting stays with the caller**: engines format values differently, +so this module does not impose a format. A value that is a ``str`` is written +as-is, ``None`` is written as an empty field, and anything else is converted +via ``str(value)``. + +Usage example:: + + from powderline.reports import write_refined_parameters, REFINED_PARAMETERS_COLUMNS + + rows = [ + { + "parameter_name": ":0:Scale", + "descriptive_name": "scale", + "phase_name": "", + "phase_idx": None, + "atom_name": "", + "atom_idx": None, + "value": "1.234567e+00", + "esd": "2.345678e-03", + "category": "scale", + }, + ] + path = write_refined_parameters(output_dir, rows) +""" + +from __future__ import annotations + +import csv +from pathlib import Path +from typing import Iterable, Mapping, Sequence, Union + +#: Column order for refined_parameters.csv (9 columns). +REFINED_PARAMETERS_COLUMNS = ( + "parameter_name", + "descriptive_name", + "phase_name", + "phase_idx", + "atom_name", + "atom_idx", + "value", + "esd", + "category", +) + +#: Column order for _unit_cell_report.csv (3 columns). +UNIT_CELL_COLUMNS = ("parameter", "value", "esd") + +#: Column order for fit_profile.txt (8 columns, tab-delimited). +FIT_PROFILE_COLUMNS = ( + "two_theta", + "y_obs", + "y_weights", + "y_calc", + "y_diff", + "y_bkg", + "q_values", + "d_spacings", +) + + +def _write_table( + path: Path, + columns: Sequence[str], + rows: Iterable[Union[Mapping[str, object], Sequence[object]]], + delimiter: str, +) -> None: + """Write a table to disk with the specified columns, rows, and delimiter. + + Args: + path: Output file path. + columns: Column names in order. + rows: Iterable of mappings (column -> value) or sequences (in column order). + delimiter: Field delimiter (',' or '\\t'). + + Notes: + - Values are written as-is if already strings, None becomes empty, else str(value). + - UTF-8 encoding with LF line endings on all platforms. + - CSV QUOTE_MINIMAL (only when necessary). + """ + path.parent.mkdir(parents=True, exist_ok=True) + + with path.open("w", encoding="utf-8", newline="") as f: + writer = csv.writer(f, delimiter=delimiter, lineterminator="\n") + writer.writerow(columns) + + for row in rows: + if isinstance(row, Mapping): + values = [row.get(col) for col in columns] + else: + values = list(row) + if len(values) != len(columns): + raise ValueError( + f"{path.name}: row has {len(values)} values, expected " + f"{len(columns)} ({', '.join(columns)})" + ) + + # Convert values: str as-is, None -> "", else str(value) + formatted = [ + "" if v is None else (v if isinstance(v, str) else str(v)) + for v in values + ] + writer.writerow(formatted) + + +def write_refined_parameters( + output_dir: Path, + rows: Iterable[Union[Mapping[str, object], Sequence[object]]], +) -> Path: + """Write refined_parameters.csv with the standardized 9-column schema. + + Args: + output_dir: Directory to write the file (created if needed). + rows: Iterable of mappings (column -> value) or sequences in + REFINED_PARAMETERS_COLUMNS order. + + Returns: + Path to the written file (output_dir / "refined_parameters.csv"). + """ + path = Path(output_dir) / "refined_parameters.csv" + _write_table(path, REFINED_PARAMETERS_COLUMNS, rows, delimiter=",") + return path + + +def write_unit_cell_report( + output_dir: Path, + phase_name: str, + rows: Iterable[Union[Mapping[str, object], Sequence[object]]], +) -> Path: + """Write _unit_cell_report.csv with the 3-column schema. + + Args: + output_dir: Directory to write the file (created if needed). + phase_name: Phase name (used in the filename). + rows: Iterable of mappings (column -> value) or sequences in + UNIT_CELL_COLUMNS order. + + Returns: + Path to the written file (output_dir / "_unit_cell_report.csv"). + """ + path = Path(output_dir) / f"{phase_name}_unit_cell_report.csv" + _write_table(path, UNIT_CELL_COLUMNS, rows, delimiter=",") + return path + + +def write_fit_profile( + output_dir: Path, + rows: Iterable[Union[Mapping[str, object], Sequence[object]]], +) -> Path: + """Write fit_profile.txt with the 8-column tab-delimited schema. + + Args: + output_dir: Directory to write the file (created if needed). + rows: Iterable of mappings (column -> value) or sequences in + FIT_PROFILE_COLUMNS order. + + Returns: + Path to the written file (output_dir / "fit_profile.txt"). + """ + path = Path(output_dir) / "fit_profile.txt" + _write_table(path, FIT_PROFILE_COLUMNS, rows, delimiter="\t") + return path + + +def write_peak_list_report( + output_dir: Path, + phase_name: str, + rows: Iterable[Union[Mapping[str, object], Sequence[object]]], + columns: Sequence[str], +) -> Path: + """Write _peak_list_report.csv with engine-specific columns. + + Args: + output_dir: Directory to write the file (created if needed). + phase_name: Phase name (used in the filename). + rows: Iterable of mappings (column -> value) or sequences in + the columns order. + columns: Column names in order (varies per engine). + + Returns: + Path to the written file (output_dir / "_peak_list_report.csv"). + """ + path = Path(output_dir) / f"{phase_name}_peak_list_report.csv" + _write_table(path, columns, rows, delimiter=",") + return path diff --git a/tests/test_reports.py b/tests/test_reports.py new file mode 100644 index 0000000..2849249 --- /dev/null +++ b/tests/test_reports.py @@ -0,0 +1,399 @@ +"""Unit tests for powderline.reports (shared report writers). + +Tests cover byte-identical round-trip against committed GSAS-II references, +mapping vs. sequence row handling, None → empty field, UTF-8 non-ASCII, LF-only +line endings, output_dir creation, and the engine-free guarantee (no GSASII import). +""" + +from __future__ import annotations + +import csv +import subprocess +import sys +from pathlib import Path + +import pytest + +from powderline import reports + + +# Discover all reference files in examples/*/output/ (excluding topas/ subdirs) +_EXAMPLES_ROOT = Path(__file__).parent.parent / "examples" + + +def _discover_reference_files(pattern: str) -> list[Path]: + """Find all files matching pattern in examples/*/output/, excluding topas/.""" + found = [] + for path in _EXAMPLES_ROOT.rglob(pattern): + # Exclude files under topas/ subdirectories + if "topas" in path.parts: + continue + # Only include files under output/ directories + if "output" in path.parts: + found.append(path) + return sorted(found) + + +_REFINED_PARAMS_FILES = _discover_reference_files("refined_parameters.csv") +_UNIT_CELL_FILES = _discover_reference_files("*_unit_cell_report.csv") +_FIT_PROFILE_FILES = _discover_reference_files("fit_profile.txt") +_PEAK_LIST_FILES = _discover_reference_files("*_peak_list_report.csv") + + +# --- Byte round-trip tests --- + + +@pytest.mark.parametrize( + "ref_path", + _REFINED_PARAMS_FILES, + ids=[str(p.relative_to(_EXAMPLES_ROOT)) for p in _REFINED_PARAMS_FILES], +) +def test_refined_parameters_roundtrip(ref_path, tmp_path): + """Byte-identical round-trip for refined_parameters.csv.""" + original_bytes = ref_path.read_bytes() + + # Read with csv module as strings + with ref_path.open("r", encoding="utf-8", newline="") as f: + reader = csv.DictReader(f) + rows = list(reader) + + # Write through the shared writer + written_path = reports.write_refined_parameters(tmp_path, rows) + written_bytes = written_path.read_bytes() + + assert written_bytes == original_bytes, ( + f"Round-trip failed for {ref_path.name}. " + f"First difference at byte {_first_diff_idx(original_bytes, written_bytes)}" + ) + + +@pytest.mark.parametrize( + "ref_path", + _UNIT_CELL_FILES, + ids=[str(p.relative_to(_EXAMPLES_ROOT)) for p in _UNIT_CELL_FILES], +) +def test_unit_cell_report_roundtrip(ref_path, tmp_path): + """Byte-identical round-trip for *_unit_cell_report.csv.""" + original_bytes = ref_path.read_bytes() + + # Extract phase name from filename (remove _unit_cell_report.csv suffix) + phase_name = ref_path.stem.replace("_unit_cell_report", "") + + # Read with csv module as strings + with ref_path.open("r", encoding="utf-8", newline="") as f: + reader = csv.DictReader(f) + rows = list(reader) + + # Write through the shared writer + written_path = reports.write_unit_cell_report(tmp_path, phase_name, rows) + written_bytes = written_path.read_bytes() + + assert written_bytes == original_bytes, ( + f"Round-trip failed for {ref_path.name}. " + f"First difference at byte {_first_diff_idx(original_bytes, written_bytes)}" + ) + + +@pytest.mark.parametrize( + "ref_path", + _FIT_PROFILE_FILES, + ids=[str(p.relative_to(_EXAMPLES_ROOT)) for p in _FIT_PROFILE_FILES], +) +def test_fit_profile_roundtrip(ref_path, tmp_path): + """Byte-identical round-trip for fit_profile.txt (tab-delimited).""" + original_bytes = ref_path.read_bytes() + + # Read with csv module as strings (tab-delimited) + with ref_path.open("r", encoding="utf-8", newline="") as f: + reader = csv.DictReader(f, delimiter="\t") + rows = list(reader) + + # Write through the shared writer + written_path = reports.write_fit_profile(tmp_path, rows) + written_bytes = written_path.read_bytes() + + assert written_bytes == original_bytes, ( + f"Round-trip failed for {ref_path.name}. " + f"First difference at byte {_first_diff_idx(original_bytes, written_bytes)}" + ) + + +@pytest.mark.parametrize( + "ref_path", + _PEAK_LIST_FILES, + ids=[str(p.relative_to(_EXAMPLES_ROOT)) for p in _PEAK_LIST_FILES], +) +def test_peak_list_report_roundtrip(ref_path, tmp_path): + """Byte-identical round-trip for *_peak_list_report.csv.""" + original_bytes = ref_path.read_bytes() + + # Extract phase name from filename (remove _peak_list_report.csv suffix) + phase_name = ref_path.stem.replace("_peak_list_report", "") + + # Read with csv module as strings + with ref_path.open("r", encoding="utf-8", newline="") as f: + reader = csv.DictReader(f) + columns = reader.fieldnames + rows = list(reader) + + # Write through the shared writer + written_path = reports.write_peak_list_report(tmp_path, phase_name, rows, columns) + written_bytes = written_path.read_bytes() + + assert written_bytes == original_bytes, ( + f"Round-trip failed for {ref_path.name}. " + f"First difference at byte {_first_diff_idx(original_bytes, written_bytes)}" + ) + + +def _first_diff_idx(a: bytes, b: bytes) -> int: + """Return the index of the first differing byte, or -1 if equal.""" + for i, (x, y) in enumerate(zip(a, b)): + if x != y: + return i + if len(a) != len(b): + return min(len(a), len(b)) + return -1 + + +# --- Mapping vs sequence rows --- + + +def test_refined_parameters_mapping_vs_sequence(tmp_path): + """Mapping rows and sequence rows produce identical output.""" + # Write using mapping rows + mapping_rows = [ + { + "parameter_name": "test_param", + "descriptive_name": "Test Parameter", + "phase_name": "TestPhase", + "phase_idx": "1", + "atom_name": "TestAtom", + "atom_idx": "2", + "value": "1.234", + "esd": "0.056", + "category": "test_category", + } + ] + path1 = reports.write_refined_parameters(tmp_path / "mapping", mapping_rows) + + # Write using sequence rows (in REFINED_PARAMETERS_COLUMNS order) + sequence_rows = [ + ["test_param", "Test Parameter", "TestPhase", "1", "TestAtom", "2", "1.234", "0.056", "test_category"] + ] + path2 = reports.write_refined_parameters(tmp_path / "sequence", sequence_rows) + + assert path1.read_bytes() == path2.read_bytes() + + +def test_unit_cell_mapping_vs_sequence(tmp_path): + """Mapping rows and sequence rows produce identical output.""" + mapping_rows = [{"parameter": "cell_a", "value": "4.123", "esd": "0.001"}] + path1 = reports.write_unit_cell_report(tmp_path / "mapping", "Phase1", mapping_rows) + + sequence_rows = [["cell_a", "4.123", "0.001"]] + path2 = reports.write_unit_cell_report(tmp_path / "sequence", "Phase1", sequence_rows) + + assert path1.read_bytes() == path2.read_bytes() + + +def test_fit_profile_mapping_vs_sequence(tmp_path): + """Mapping rows and sequence rows produce identical output.""" + mapping_rows = [ + { + "two_theta": "1.0", + "y_obs": "2.0", + "y_weights": "3.0", + "y_calc": "4.0", + "y_diff": "5.0", + "y_bkg": "6.0", + "q_values": "7.0", + "d_spacings": "8.0", + } + ] + path1 = reports.write_fit_profile(tmp_path / "mapping", mapping_rows) + + sequence_rows = [["1.0", "2.0", "3.0", "4.0", "5.0", "6.0", "7.0", "8.0"]] + path2 = reports.write_fit_profile(tmp_path / "sequence", sequence_rows) + + assert path1.read_bytes() == path2.read_bytes() + + +def test_peak_list_mapping_vs_sequence(tmp_path): + """Mapping rows and sequence rows produce identical output.""" + columns = ["h", "k", "l", "d_spacing"] + mapping_rows = [{"h": "1", "k": "2", "l": "3", "d_spacing": "4.56"}] + path1 = reports.write_peak_list_report(tmp_path / "mapping", "Phase1", mapping_rows, columns) + + sequence_rows = [["1", "2", "3", "4.56"]] + path2 = reports.write_peak_list_report(tmp_path / "sequence", "Phase1", sequence_rows, columns) + + assert path1.read_bytes() == path2.read_bytes() + + +# --- None → empty field --- + + +def test_none_becomes_empty_field(tmp_path): + """None values are written as empty fields.""" + rows = [ + { + "parameter_name": "param1", + "descriptive_name": None, + "phase_name": "", + "phase_idx": None, + "atom_name": None, + "atom_idx": None, + "value": "1.0", + "esd": None, + "category": "test", + } + ] + path = reports.write_refined_parameters(tmp_path, rows) + content = path.read_text(encoding="utf-8") + + # Check the data row (second line after header) + lines = content.strip().split("\n") + assert len(lines) == 2 + data_line = lines[1] + # descriptive_name (None), phase_name (""), phase_idx (None), atom_name (None), atom_idx (None), esd (None) should be empty + assert data_line == "param1,,,,,,1.0,,test" + + +# --- LF-only line endings --- + + +def test_lf_only_no_crlf(tmp_path): + """Output files contain only LF line endings, never CRLF.""" + rows = [{"parameter": "cell_a", "value": "4.0", "esd": "0.001"}] + path = reports.write_unit_cell_report(tmp_path, "TestPhase", rows) + content_bytes = path.read_bytes() + + assert b"\r" not in content_bytes, "File contains CRLF (\\r\\n) but should be LF-only" + + +# --- UTF-8 non-ASCII round-trip --- + + +def test_utf8_non_ascii_roundtrip(tmp_path): + """UTF-8 non-ASCII characters (e.g., subscripts) round-trip correctly.""" + rows = [ + { + "parameter_name": "test_param", + "descriptive_name": "Test", + "phase_name": "Fe₂O₃", # Unicode subscripts + "phase_idx": "1", + "atom_name": "Fe", + "atom_idx": "1", + "value": "1.0", + "esd": "0.01", + "category": "test", + } + ] + path = reports.write_refined_parameters(tmp_path, rows) + + # Read back and verify the phase name + with path.open("r", encoding="utf-8", newline="") as f: + reader = csv.DictReader(f) + row = next(reader) + assert row["phase_name"] == "Fe₂O₃" + + +# --- output_dir creation --- + + +def test_output_dir_created(tmp_path): + """Output directory is created if it doesn't exist.""" + nested_dir = tmp_path / "nested" / "path" / "output" + assert not nested_dir.exists() + + rows = [{"parameter": "cell_a", "value": "4.0", "esd": "0.001"}] + path = reports.write_unit_cell_report(nested_dir, "Phase1", rows) + + assert nested_dir.exists() + assert path.exists() + assert path == nested_dir / "Phase1_unit_cell_report.csv" + + +# --- Engine-free guarantee (no GSASII import) --- + + +def test_no_gsasii_import(): + """Importing powderline.reports does not import GSASII.""" + # Run in a subprocess to get a fresh import environment + code = """ +import sys +import importlib.abc + +# Block GSASII import using MetaPathFinder +class _BlockGSASII(importlib.abc.MetaPathFinder): + def find_spec(self, name, path, target=None): + if name == "GSASII" or name.startswith("GSASII."): + raise ImportError(f"Blocked import of {name} (should not be needed)") + return None + +sys.meta_path.insert(0, _BlockGSASII()) + +# Now try to import powderline.reports +import powderline.reports + +# Verify GSASII was not imported +assert "GSASII" not in sys.modules, "GSASII was imported (should not happen)" +print("OK") +""" + result = subprocess.run( + [sys.executable, "-c", code], + cwd=Path(__file__).parent.parent, + env={**subprocess.os.environ, "PYTHONPATH": str(Path(__file__).parent.parent / "src")}, + capture_output=True, + text=True, + ) + + assert result.returncode == 0, ( + f"Subprocess failed:\n" + f"STDOUT: {result.stdout}\n" + f"STDERR: {result.stderr}" + ) + assert "OK" in result.stdout + + +# --- Column constants --- + + +def test_refined_parameters_columns_constant(): + """REFINED_PARAMETERS_COLUMNS has the expected 9 columns in order.""" + assert reports.REFINED_PARAMETERS_COLUMNS == ( + "parameter_name", + "descriptive_name", + "phase_name", + "phase_idx", + "atom_name", + "atom_idx", + "value", + "esd", + "category", + ) + + +def test_unit_cell_columns_constant(): + """UNIT_CELL_COLUMNS has the expected 3 columns in order.""" + assert reports.UNIT_CELL_COLUMNS == ("parameter", "value", "esd") + + +def test_fit_profile_columns_constant(): + """FIT_PROFILE_COLUMNS has the expected 8 columns in order.""" + assert reports.FIT_PROFILE_COLUMNS == ( + "two_theta", + "y_obs", + "y_weights", + "y_calc", + "y_diff", + "y_bkg", + "q_values", + "d_spacings", + ) + + +def test_sequence_row_length_mismatch_raises(tmp_path): + with pytest.raises(ValueError, match="row has 2 values, expected 3"): + reports.write_unit_cell_report(tmp_path, "LaB6", [("a", "1.0")]) From 8d98e6dc92a285ea6c9a5d273eb1d8692ab793f9 Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:56:27 -0400 Subject: [PATCH 06/10] feat(core): gateway registry + engine-free gateway factories; dispatcher routed through it - powderline.registry: lazy discovery of the 'powderline.gateways' entry-point group (ep.load on first get), register_gateway() escape hatch, GatewayNotInstalledError, and a clear stale-install-metadata error when the group is empty. Gateway = frozen dataclass (A54); check_engine_version() enforces the PEP 440 ENGINE_VERSION_SPEC as a hard error (A44). - Engine-free gateway.py per gateway (gsasii, topas, easydiff): schema layer validate()/capabilities() runs with no engine installed (A50); runtime run() wraps today's kicker.run / run_topas_recipe / run_easydiffraction_recipe unchanged, importing the engine lazily. Real runs check the engine version (GSASII.__version__; configured TOPAS version, skipped when unset; easydiffraction distribution version) with a gateway-level allow_unsupported_engine_version bypass (A53); a missing engine raises EngineNotAvailableError (an ImportError, A52). Specs: GSAS-II ==5.7.9, TOPAS >=7,<8, easydiffraction >=0.20.1,<0.21. - powderline.engine.run: old engine= names map 1:1 to registry names; public signature and per-engine outputs unchanged (LaB6 gsasii + topas outputs byte-identical to ae415c8). - pyproject: [project.entry-points."powderline.gateways"]; packaging>=22 declared in pyproject + pixi.toml (lock: requires_dist only). CLAUDE.md notes the reinstall needed after entry-point edits. - Tests: registry (escape hatch, laziness, error texts, real entry points, PYTHONPATH=src mode) + gateways (specs, validate, version checks, dispatcher forwarding, import-block schema-only path with GSAS-II/easydiffraction blocked). Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 4 + pixi.lock | 1 + pixi.toml | 1 + pyproject.toml | 10 + src/powderline/easydiff/gateway.py | 74 +++ src/powderline/engine.py | 33 +- src/powderline/gateways/gsasii/__init__.py | 2 +- src/powderline/gateways/gsasii/gateway.py | 76 +++ src/powderline/registry.py | 148 ++++++ src/powderline/topas/gateway.py | 72 +++ tests/test_gateways.py | 524 +++++++++++++++++++++ tests/test_registry.py | 284 +++++++++++ 12 files changed, 1209 insertions(+), 20 deletions(-) create mode 100644 src/powderline/easydiff/gateway.py create mode 100644 src/powderline/gateways/gsasii/gateway.py create mode 100644 src/powderline/registry.py create mode 100644 src/powderline/topas/gateway.py create mode 100644 tests/test_gateways.py create mode 100644 tests/test_registry.py diff --git a/CLAUDE.md b/CLAUDE.md index 8e5b8e2..045b204 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -105,6 +105,10 @@ The 6-name public API is in `src/powderline/__init__.py` `__all__`: - `pixi run mp-simulate --material-id mp-2680` — simulate a pattern from Materials Project (`--formula`, `--wavelength`, `--no-server`, `--keep-recipe`). - `pixi run docs` / `pixi run docs-clean` — build / clean the Sphinx HTML docs. +- `pixi reinstall powderline` — required after editing `[project.entry-points]` in + `pyproject.toml`: entry-point metadata is written at install time, and the gateway + registry (`powderline.registry`, group `powderline.gateways`) finds nothing until + the editable package is re-installed. - `pixi run update-code-hash` — regenerate `src/powderline/_code_hash.json` after editing `kicker.py` (guarded by `tests/test_code_hash.py`). diff --git a/pixi.lock b/pixi.lock index a4e13ee..b51a946 100644 --- a/pixi.lock +++ b/pixi.lock @@ -17366,6 +17366,7 @@ packages: - h5py>=3.12 - httpx>=0.28 - numpy>=2.0 + - packaging>=22 - pandas>=2.0 - pybaselines>=1.2 - pydantic>=2.0 diff --git a/pixi.toml b/pixi.toml index 27d717e..834faf8 100644 --- a/pixi.toml +++ b/pixi.toml @@ -59,6 +59,7 @@ fastapi = ">=0.115.0,<1" uvicorn = ">=0.34.0,<1" httpx = ">=0.28.0,<1" gemmi = ">=0.7.5,<0.8" +packaging = ">=22" # GSAS-II is built as a conda package via pixi-build (see `preview` above) from # the tacaswell fork, which adds the conda recipe. This installs GSAS-II on ALL # platforms (linux-64/win-64/osx-arm64), unlike the former linux-64-only pip pin. diff --git a/pyproject.toml b/pyproject.toml index 4af4d36..b0fc914 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -52,8 +52,18 @@ dependencies = [ "uvicorn>=0.34", "httpx>=0.28", "gemmi>=0.7,<0.8", + "packaging>=22", ] +# Gateway plugins (master plan A17): each entry targets an engine-free factory +# module. Entry-point metadata is written at install time -- after editing this +# table, re-install the editable package (`pixi reinstall powderline`) or the +# registry finds no gateways. +[project.entry-points."powderline.gateways"] +gsasii = "powderline.gateways.gsasii.gateway:get_gateway" +topas = "powderline.topas.gateway:get_gateway" +easydiffraction = "powderline.easydiff.gateway:get_gateway" + [project.urls] Homepage = "https://github.com/NSLS2/PowderLine" Documentation = "https://nsls2.github.io/PowderLine/" diff --git a/src/powderline/easydiff/gateway.py b/src/powderline/easydiff/gateway.py new file mode 100644 index 0000000..dae0738 --- /dev/null +++ b/src/powderline/easydiff/gateway.py @@ -0,0 +1,74 @@ +"""Engine-free factory for the easydiffraction gateway (entry point +``powderline.gateways:easydiffraction``). + +- **Schema layer** — ``validate`` / ``capabilities``: builds the ``RecipeModel`` + then runs the policy pre-flight, exactly the check ``validate_only`` runs today + (raises ``EasyDiffractionTranslationError``; no easydiffraction import). +- **Runtime layer** — ``run``: a thin wrapper over + ``powderline.easydiff.engine.run_easydiffraction_recipe`` (behavior unchanged). + On a real run it first reads the installed easydiffraction version: missing ⇒ + ``EngineNotAvailableError``; outside ``ENGINE_VERSION_SPEC`` ⇒ + ``EngineVersionError``. +""" + +from __future__ import annotations + +from powderline.exceptions import EngineNotAvailableError +from powderline.registry import Gateway, check_engine_version +from powderline.schema import RecipeModel + +NAME = "easydiffraction" +#: Supported easydiffraction versions (A41/A44); mirrors the pixi pin. +ENGINE_VERSION_SPEC = ">=0.20.1,<0.21" +#: Workflows served (0.26.0 schema name until re/06; no SPF — A13.3). +WORKFLOWS = ("GSASII_Rietveld",) +#: pixi environment providing easydiffraction + how to install it (renamed in re/08, A42). +ENV = "easydiff" +INSTALL_COMMAND = "pixi install -e easydiff" + + +def capabilities() -> dict: + return {"name": NAME, "workflows": WORKFLOWS, "engine_version_spec": ENGINE_VERSION_SPEC} + + +def validate(recipe): + """Validate a recipe (dict or ``RecipeModel``); return the ``RecipeModel`` (A50).""" + from powderline.easydiff.policy import check_unsupported + + model = recipe if isinstance(recipe, RecipeModel) else RecipeModel.model_validate(recipe) + check_unsupported(model.model_dump()) # may raise EasyDiffractionTranslationError + return model + + +def installed_engine_version() -> str: + """The installed easydiffraction version (distribution == runtime there).""" + from importlib.metadata import PackageNotFoundError, version + + try: + return version("easydiffraction") + except PackageNotFoundError as exc: + raise EngineNotAvailableError(gateway=NAME, dependency="easydiffraction", env=ENV, + install_command=INSTALL_COMMAND) from exc + + +def run(recipe, output_dir, *, verbose: bool = False, validate_only: bool = False, + allow_unsupported_engine_version: bool = False) -> dict: + """Run through ``run_easydiffraction_recipe``; see its docstring for the result dict. + + ``allow_unsupported_engine_version`` is the test-only bypass of the engine + version check (A53); the check is skipped for ``validate_only``. + """ + if not validate_only: + check_engine_version(gateway=NAME, engine="easydiffraction", + installed=installed_engine_version(), spec=ENGINE_VERSION_SPEC, + allow_unsupported=allow_unsupported_engine_version) + # Bind the module (not the function) so tests can monkeypatch + # ``run_easydiffraction_recipe`` on it. + from powderline.easydiff import engine as _easydiff_engine + + return _easydiff_engine.run_easydiffraction_recipe(recipe, output_dir, verbose=verbose, + validate_only=validate_only) + + +def get_gateway() -> Gateway: + return Gateway(name=NAME, capabilities=capabilities, validate=validate, run=run) diff --git a/src/powderline/engine.py b/src/powderline/engine.py index 9619112..3b7ee3d 100644 --- a/src/powderline/engine.py +++ b/src/powderline/engine.py @@ -7,11 +7,19 @@ time — the GSAS-II backend is imported lazily only for ``engine="gsasii"``, so ``powderline.run(engine="topas")`` or ``powderline.run(engine="easydiffraction")`` works on machines without GSAS-II. + +Dispatch goes through the gateway registry (``powderline.registry``, entry-point +group ``powderline.gateways``); each old ``engine=`` name maps 1:1 to a registry +name. Each gateway's runtime layer imports its engine lazily. """ from __future__ import annotations +from powderline import registry + _ENGINES = ("gsasii", "topas", "easydiffraction") +#: Old ``engine=`` names -> registry names (1:1; the ``engine=`` argument is removed in re/07). +_ENGINE_TO_GATEWAY = {"gsasii": "gsasii", "topas": "topas", "easydiffraction": "easydiffraction"} def run( @@ -42,10 +50,11 @@ def run( The return dict shape is identical across engines (see ``tests/test_api.py``). """ + if engine not in _ENGINES: + raise ValueError(f"unknown engine {engine!r}; expected one of {_ENGINES}") + gateway = registry.get(_ENGINE_TO_GATEWAY[engine]) if engine == "gsasii": - from powderline.kicker import run as _gsas_run # lazy: imports GSAS-II - - return _gsas_run( + return gateway.run( recipe, output_dir, verbose=verbose, @@ -53,9 +62,7 @@ def run( execution_mode=execution_mode, ) if engine == "topas": - from powderline.topas.engine import run_topas_recipe # GSAS-II-free - - return run_topas_recipe( + return gateway.run( recipe, output_dir, verbose=verbose, @@ -63,16 +70,4 @@ def run( topas_dir=topas_dir, topas_version=topas_version, ) - if engine == "easydiffraction": - # GSAS-II-free; lazy import so this dispatcher loads without - # easydiffraction installed. Bind the module (not the function) so tests - # can monkeypatch ``run_easydiffraction_recipe`` on it. - from powderline.easydiff import engine as _easydiff_engine - - return _easydiff_engine.run_easydiffraction_recipe( - recipe, - output_dir, - verbose=verbose, - validate_only=validate_only, - ) - raise ValueError(f"unknown engine {engine!r}; expected one of {_ENGINES}") + return gateway.run(recipe, output_dir, verbose=verbose, validate_only=validate_only) diff --git a/src/powderline/gateways/gsasii/__init__.py b/src/powderline/gateways/gsasii/__init__.py index d1444ad..84e84f4 100644 --- a/src/powderline/gateways/gsasii/__init__.py +++ b/src/powderline/gateways/gsasii/__init__.py @@ -2,5 +2,5 @@ Do not import runtime modules (setters, project, executors, extractors, ...) here: they import GSAS-II at module top. The engine-free gateway factory -arrives in a later branch (re/02) as ``gateways/gsasii/gateway.py``. +(the ``powderline.gateways`` entry point) is ``gateways/gsasii/gateway.py``. """ diff --git a/src/powderline/gateways/gsasii/gateway.py b/src/powderline/gateways/gsasii/gateway.py new file mode 100644 index 0000000..1cab87b --- /dev/null +++ b/src/powderline/gateways/gsasii/gateway.py @@ -0,0 +1,76 @@ +"""Engine-free factory for the GSAS-II gateway (entry point ``powderline.gateways:gsasii``). + +- **Schema layer** — ``validate`` / ``capabilities``: engine-free; runs with + GSAS-II not installed (``gateways.gsasii.validation`` is recipe-only). +- **Runtime layer** — ``run``: a thin wrapper over ``powderline.kicker.run`` + (behavior unchanged), imported inside the function. On a real run it first + reads the installed GSAS-II version and enforces ``ENGINE_VERSION_SPEC``. + +Never import ``powderline.kicker`` or the gateway's runtime modules (helpers, +project, setters, executors, extractors) at module top: they import GSAS-II. +Enforced by an import-block test. +""" + +from __future__ import annotations + +from powderline.exceptions import EngineNotAvailableError +from powderline.gateways.gsasii.validation import validate as _validate_recipe +from powderline.registry import Gateway, check_engine_version + +NAME = "gsasii" +#: Supported GSAS-II runtime versions (A41/A44); pixi pins align in re/08. +ENGINE_VERSION_SPEC = "==5.7.9" +#: Workflows served (0.26.0 schema names until re/04 introduces ``gsasii.*``). +WORKFLOWS = ("GSASII_Rietveld", "GSASII_SPF") +#: pixi environment providing GSAS-II + how to install it (names final in re/08). +ENV = "default" +INSTALL_COMMAND = "pixi install" + + +def capabilities() -> dict: + return {"name": NAME, "workflows": WORKFLOWS, "engine_version_spec": ENGINE_VERSION_SPEC} + + +def validate(recipe, *, verbose: bool = False): + """Validate a recipe (dict or ``RecipeModel``); return the ``RecipeModel`` (A50).""" + return _validate_recipe(recipe, verbose=verbose) + + +def installed_engine_version() -> str: + """The GSAS-II **runtime** version (``GSASII.__version__``), never distribution metadata. + + The pixi-built conda distribution reports ``gsas-ii 2.0.0`` (the fork recipe's + version); the runtime reports ``5.7.9``. Fallback: ``GSASIIpath.GetVersionTag()`` + (``v5.7.9``) with the ``v`` stripped. + """ + try: + import GSASII + except ImportError as exc: + raise EngineNotAvailableError(gateway=NAME, dependency="GSAS-II", env=ENV, + install_command=INSTALL_COMMAND) from exc + version = getattr(GSASII, "__version__", None) + if not version: + from GSASII import GSASIIpath + version = GSASIIpath.GetVersionTag().lstrip("v") + return str(version) + + +def run(recipe, output_dir, *, verbose: bool = False, validate_only: bool = False, + execution_mode: str = "auto", allow_unsupported_engine_version: bool = False) -> dict: + """Run through ``powderline.kicker.run``; see its docstring for the result dict. + + ``allow_unsupported_engine_version`` is the test-only bypass of the engine + version check (A53); the check is skipped for ``validate_only``. + """ + if not validate_only: + check_engine_version(gateway=NAME, engine="GSAS-II", installed=installed_engine_version(), + spec=ENGINE_VERSION_SPEC, + allow_unsupported=allow_unsupported_engine_version) + from powderline.kicker import run as _gsas_run # lazy: imports GSAS-II + + return _gsas_run(recipe, output_dir, verbose=verbose, validate_only=validate_only, + execution_mode=execution_mode) + + +def get_gateway() -> Gateway: + return Gateway(name=NAME, capabilities=capabilities, validate=validate, run=run) diff --git a/src/powderline/registry.py b/src/powderline/registry.py new file mode 100644 index 0000000..d2163d8 --- /dev/null +++ b/src/powderline/registry.py @@ -0,0 +1,148 @@ +"""Gateway registry: discovers PowderLine gateways from the ``powderline.gateways`` +entry-point group (master plan A17, A7). + +Discovery is lazy: entry points are listed on first use and a gateway's +factory is imported (``ep.load()``) only when that gateway is first requested, +so importing this module (or ``powderline``) never imports an engine. + +Each entry point targets an engine-free ``get_gateway`` factory returning a +:class:`Gateway` with two layers: the **schema layer** (``validate``, +``capabilities``) runs with no engine installed; the **runtime layer** +(``run``) imports its engine lazily and raises +:class:`~powderline.exceptions.EngineNotAvailableError` when it is missing. + +``register_gateway(name, factory)`` is the explicit-registration escape hatch +for tests and development; an explicit registration takes precedence over an +entry point of the same name. + +Entry-point metadata is written when the package is installed: after editing +``[project.entry-points]`` in ``pyproject.toml``, re-install the editable +package (``pixi reinstall powderline``) or discovery finds nothing. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from importlib.metadata import entry_points +from typing import Any, Callable, Dict, Optional + +from packaging.specifiers import SpecifierSet +from packaging.version import InvalidVersion, Version + +from powderline.exceptions import EngineVersionError, GatewayNotInstalledError + +ENTRY_POINT_GROUP = "powderline.gateways" + + +@dataclass(frozen=True) +class Gateway: + """A gateway's contract (A54). + + - ``name``: registry name (``"gsasii"``, ``"topas"``, ``"easydiffraction"``). + - ``capabilities()``: dict with at least ``name``, ``workflows`` and + ``engine_version_spec`` (a PEP 440 specifier string, A41). + - ``validate(recipe, **opts)``: schema layer, engine-free. Takes a recipe + dict or an already-validated model, raises on an invalid recipe, returns + the validated model (A50). + - ``run(recipe, output_dir, **opts)``: runtime layer; imports the engine + lazily; returns the standardized result dict. + """ + + name: str + capabilities: Callable[[], Dict[str, Any]] + validate: Callable[..., Any] + run: Callable[..., dict] + + +_explicit: Dict[str, Callable[[], Gateway]] = {} +_entry_points: Optional[Dict[str, Any]] = None +_instances: Dict[str, Gateway] = {} + + +def _discovered() -> Dict[str, Any]: + """Entry points of the ``powderline.gateways`` group, listed once (not loaded).""" + global _entry_points + if _entry_points is None: + _entry_points = {ep.name: ep for ep in entry_points(group=ENTRY_POINT_GROUP)} + return _entry_points + + +def register_gateway(name: str, factory: Callable[[], Gateway]) -> None: + """Register a gateway factory explicitly (tests/dev escape hatch). + + Takes precedence over an entry point of the same name. + """ + _explicit[name] = factory + _instances.pop(name, None) + + +def unregister_gateway(name: str) -> None: + """Remove an explicit registration (entry-point gateways are unaffected).""" + _explicit.pop(name, None) + _instances.pop(name, None) + + +def available() -> list[str]: + """Sorted names of every registered gateway (explicit and entry-point).""" + return sorted(set(_explicit) | set(_discovered())) + + +def get(name: str) -> Gateway: + """Return the gateway registered as ``name``, loading its factory on first use. + + Raises: + GatewayNotInstalledError: no gateway of that name is registered at all. + When the entry-point group is empty the message says the install + metadata is stale (the in-repo gateways are always declared). + """ + if name in _instances: + return _instances[name] + if name in _explicit: + factory = _explicit[name] + else: + eps = _discovered() + if name not in eps: + raise GatewayNotInstalledError(_not_installed_message(name, eps), gateway=name) + factory = eps[name].load() + gateway = factory() + if not isinstance(gateway, Gateway): + raise TypeError(f"gateway factory for {name!r} returned {type(gateway).__name__}, " + "expected powderline.registry.Gateway") + _instances[name] = gateway + return gateway + + +def _not_installed_message(name: str, eps: Dict[str, Any]) -> str: + if not eps: + return ( + f"no PowderLine gateway named {name!r}: no {ENTRY_POINT_GROUP!r} entry " + "points were found at all, although powderline is importable. The " + "package's install metadata is stale or missing; re-install it " + "(`pixi reinstall powderline`, or `pip install -e .`)." + ) + return ( + f"no PowderLine gateway named {name!r} is installed (available: " + f"{', '.join(available())}). Gateways are discovered from the " + f"{ENTRY_POINT_GROUP!r} entry-point group: install the package that " + "provides it (e.g. `pip install `), or register it with " + "powderline.registry.register_gateway()." + ) + + +def check_engine_version(*, gateway: str, engine: str, installed, spec: str, + allow_unsupported: bool = False) -> None: + """Raise :class:`EngineVersionError` unless ``installed`` satisfies ``spec`` (A44). + + A hard error. ``allow_unsupported=True`` is the explicit test-only bypass + for compatibility testing of candidate engine versions (A53); gateways + expose it as ``allow_unsupported_engine_version`` on their ``run()``. + """ + if allow_unsupported: + return + try: + ok = Version(str(installed)) in SpecifierSet(spec) + except InvalidVersion: + ok = False + if not ok: + raise EngineVersionError(gateway=gateway, engine=engine, installed=str(installed), + supported=spec) diff --git a/src/powderline/topas/gateway.py b/src/powderline/topas/gateway.py new file mode 100644 index 0000000..9beb886 --- /dev/null +++ b/src/powderline/topas/gateway.py @@ -0,0 +1,72 @@ +"""Engine-free factory for the TOPAS gateway (entry point ``powderline.gateways:topas``). + +- **Schema layer** — ``validate`` / ``capabilities``: builds the ``RecipeModel`` + then renders the INP in memory, exactly the check ``validate_only`` runs + today (raises ``TopasTranslationError`` on untranslatable input). +- **Runtime layer** — ``run``: a thin wrapper over + ``powderline.topas.engine.run_topas_recipe`` (behavior unchanged). + +TOPAS has no Python engine dependency, so ``EngineNotAvailableError`` never +applies: without ``tc.exe`` the run degrades to generate-only, as today. TOPAS +also has no queryable runtime version: the version check uses the *configured* +version (``topas_version`` argument, else ``.powderline_config.yaml`` +``topas.version``) and is skipped when none is configured (A53) — it checks what +the user configured, not the binary. +""" + +from __future__ import annotations + +from powderline.registry import Gateway, check_engine_version +from powderline.schema import RecipeModel + +NAME = "topas" +#: Supported TOPAS versions (A41/A44). +ENGINE_VERSION_SPEC = ">=7,<8" +#: Workflows served (0.26.0 schema names until re/05 introduces ``topas.*``). +WORKFLOWS = ("GSASII_Rietveld", "GSASII_SPF") + + +def capabilities() -> dict: + return {"name": NAME, "workflows": WORKFLOWS, "engine_version_spec": ENGINE_VERSION_SPEC} + + +def validate(recipe): + """Validate a recipe (dict or ``RecipeModel``); return the ``RecipeModel`` (A50).""" + from powderline.topas.writer import render_topas + + model = recipe if isinstance(recipe, RecipeModel) else RecipeModel.model_validate(recipe) + render_topas(model.model_dump(), "topas") # validates translatability; may raise + return model + + +def configured_engine_version(topas_version=None): + """The configured TOPAS version, or ``None`` when none is configured.""" + if topas_version is not None: + return topas_version + from powderline.topas.runner import _read_topas_config + + return _read_topas_config().get("version") + + +def run(recipe, output_dir, *, verbose: bool = False, validate_only: bool = False, + topas_dir=None, topas_version=None, + allow_unsupported_engine_version: bool = False) -> dict: + """Run through ``run_topas_recipe``; see its docstring for the result dict. + + ``allow_unsupported_engine_version`` is the test-only bypass of the engine + version check (A53); the check is skipped for ``validate_only``. + """ + if not validate_only: + version = configured_engine_version(topas_version) + if version is not None: + check_engine_version(gateway=NAME, engine="TOPAS", installed=version, + spec=ENGINE_VERSION_SPEC, + allow_unsupported=allow_unsupported_engine_version) + from powderline.topas.engine import run_topas_recipe + + return run_topas_recipe(recipe, output_dir, verbose=verbose, validate_only=validate_only, + topas_dir=topas_dir, topas_version=topas_version) + + +def get_gateway() -> Gateway: + return Gateway(name=NAME, capabilities=capabilities, validate=validate, run=run) diff --git a/tests/test_gateways.py b/tests/test_gateways.py new file mode 100644 index 0000000..f7cd33b --- /dev/null +++ b/tests/test_gateways.py @@ -0,0 +1,524 @@ +"""Tests for the three built-in gateways: schema layer, version checking, import-block guarantee. + +Coverage: +- ENGINE_VERSION_SPEC consistency across all gateways +- validate() for each gateway (LaB6 recipe) +- Version-check behavior (gsasii, topas, easydiffraction) +- Dispatcher routing (registry.get → gateway.run) +- Import-block guarantee: capabilities/validate work with engines unavailable +""" + +from __future__ import annotations + +import importlib.util +import json +import sys +from pathlib import Path +from unittest.mock import Mock + +import pytest +from packaging.specifiers import SpecifierSet +from pydantic import ValidationError + +from powderline import registry, schema +from powderline.exceptions import EngineNotAvailableError, EngineVersionError +from powderline.gateways.gsasii import gateway as gsasii_gateway +from powderline.topas import gateway as topas_gateway +from powderline.easydiff import gateway as easydiff_gateway +from subprocess_utils import run_subprocess_utf8 + +REPO = Path(__file__).resolve().parent.parent +EXAMPLES = REPO / "examples" + + +def _lab6_recipe(): + """Load the LaB6 example recipe as a dict.""" + return json.loads((EXAMPLES / "example_LaB6" / "input.json").read_text()) + + +# --- ENGINE_VERSION_SPEC consistency --- + + +@pytest.mark.parametrize( + "gateway_module,expected_spec", + [ + (gsasii_gateway, "==5.7.9"), + (topas_gateway, ">=7,<8"), + (easydiff_gateway, ">=0.20.1,<0.21"), + ], +) +def test_engine_version_spec_parses_and_matches_capabilities(gateway_module, expected_spec): + """Each gateway's ENGINE_VERSION_SPEC parses and equals capabilities()['engine_version_spec'].""" + spec_str = gateway_module.ENGINE_VERSION_SPEC + SpecifierSet(spec_str) # must parse + assert spec_str == expected_spec + caps = gateway_module.capabilities() + assert caps["engine_version_spec"] == expected_spec + + +@pytest.mark.parametrize( + "gateway_module", + [gsasii_gateway, topas_gateway, easydiff_gateway], +) +def test_get_gateway_returns_gateway_with_correct_name(gateway_module): + """Each gateway module's get_gateway() returns a Gateway whose name == NAME.""" + gw = gateway_module.get_gateway() + assert gw.name == gateway_module.NAME + assert isinstance(gw, registry.Gateway) + + +# --- validate() --- + + +@pytest.mark.parametrize( + "gateway_module", + [gsasii_gateway, topas_gateway], +) +def test_validate_lab6_recipe_dict_returns_recipe_model(gateway_module): + """Validating the LaB6 recipe dict returns a RecipeModel. + + Note: easydiffraction is excluded because the LaB6 recipe has a Z broadening + parameter flagged for refinement, which easydiffraction does not support. + """ + recipe = _lab6_recipe() + result = gateway_module.validate(recipe) + assert isinstance(result, schema.RecipeModel) + + +@pytest.mark.parametrize( + "gateway_module", + [gsasii_gateway, topas_gateway], +) +def test_validate_recipe_model_is_idempotent(gateway_module): + """Passing a RecipeModel to validate() returns a RecipeModel (idempotent). + + Note: easydiffraction is excluded because the LaB6 recipe has a Z broadening + parameter flagged for refinement, which easydiffraction does not support. + """ + recipe = _lab6_recipe() + model = schema.RecipeModel.model_validate(recipe) + result = gateway_module.validate(model) + assert isinstance(result, schema.RecipeModel) + + +@pytest.mark.parametrize( + "gateway_module,invalid_recipe", + [ + (gsasii_gateway, {"schema_name": "GSASII_Rietveld"}), + (topas_gateway, {"schema_name": "GSASII_Rietveld"}), + (easydiff_gateway, {"schema_name": "GSASII_Rietveld"}), + (gsasii_gateway, {}), + (topas_gateway, {}), + (easydiff_gateway, {}), + ], +) +def test_validate_invalid_recipe_raises(gateway_module, invalid_recipe): + """An invalid recipe raises ValidationError (from pydantic or wrapped).""" + with pytest.raises((ValidationError, Exception)): # may be wrapped + gateway_module.validate(invalid_recipe) + + +def test_easydiff_validate_lab6_raises_translation_error(): + """Easydiffraction validate raises on the LaB6 recipe due to unsupported Z parameter.""" + from powderline.easydiff.errors import EasyDiffractionTranslationError + + recipe = _lab6_recipe() + with pytest.raises(EasyDiffractionTranslationError, match="Z is flagged for refinement"): + easydiff_gateway.validate(recipe) + + +# --- gsasii version checking --- + + +@pytest.mark.skipif( + importlib.util.find_spec("GSASII") is None, reason="GSAS-II not installed" +) +def test_gsasii_installed_engine_version_satisfies_spec(): + """When GSAS-II is installed, installed_engine_version() satisfies ENGINE_VERSION_SPEC.""" + version = gsasii_gateway.installed_engine_version() + spec = SpecifierSet(gsasii_gateway.ENGINE_VERSION_SPEC) + assert version in spec, f"installed {version} not in {spec}" + + +def test_gsasii_version_check_raises_when_version_out_of_range(tmp_path, monkeypatch): + """When the installed version is out of range, run() raises EngineVersionError.""" + monkeypatch.setattr(gsasii_gateway, "installed_engine_version", lambda: "5.8.0") + # Mock the kicker.run so it doesn't actually run + import powderline.kicker + + fake_run = Mock() + monkeypatch.setattr(powderline.kicker, "run", fake_run) + + with pytest.raises(EngineVersionError): + gsasii_gateway.run(_lab6_recipe(), str(tmp_path), validate_only=False) + assert not fake_run.called, "kicker.run should not be called when version check fails" + + +def test_gsasii_version_check_bypassed_with_allow_unsupported(tmp_path, monkeypatch): + """With allow_unsupported_engine_version=True, the version check is bypassed.""" + monkeypatch.setattr(gsasii_gateway, "installed_engine_version", lambda: "5.8.0") + import powderline.kicker + + fake_run = Mock(return_value={}) + monkeypatch.setattr(powderline.kicker, "run", fake_run) + + gsasii_gateway.run( + _lab6_recipe(), + str(tmp_path), + validate_only=False, + verbose=True, + execution_mode="auto", + allow_unsupported_engine_version=True, + ) + assert fake_run.called + call_kwargs = fake_run.call_args[1] + assert call_kwargs["verbose"] is True + assert call_kwargs["validate_only"] is False + assert call_kwargs["execution_mode"] == "auto" + + +def test_gsasii_validate_only_skips_version_check(tmp_path, monkeypatch): + """With validate_only=True, the version check is skipped.""" + called = [] + + def failing_version_check(): + called.append(True) + raise AssertionError("version check should not be called for validate_only=True") + + monkeypatch.setattr(gsasii_gateway, "installed_engine_version", failing_version_check) + import powderline.kicker + + fake_run = Mock(return_value={}) + monkeypatch.setattr(powderline.kicker, "run", fake_run) + + gsasii_gateway.run(_lab6_recipe(), str(tmp_path), validate_only=True) + assert not called, "installed_engine_version should not be called for validate_only=True" + + +# --- topas version checking --- + + +def test_topas_no_version_configured_skips_check(tmp_path, monkeypatch): + """When no version is configured, the version check is skipped.""" + from powderline.topas import runner, engine + + monkeypatch.setattr(runner, "_read_topas_config", lambda: {}) + fake_run = Mock(return_value={}) + monkeypatch.setattr(engine, "run_topas_recipe", fake_run) + + topas_gateway.run(_lab6_recipe(), str(tmp_path), validate_only=False) + assert fake_run.called + + +def test_topas_version_kwarg_out_of_range_raises(tmp_path, monkeypatch): + """topas_version=6 (out of range) raises EngineVersionError.""" + from powderline.topas import engine + + fake_run = Mock() + monkeypatch.setattr(engine, "run_topas_recipe", fake_run) + + with pytest.raises(EngineVersionError): + topas_gateway.run(_lab6_recipe(), str(tmp_path), topas_version=6, validate_only=False) + assert not fake_run.called + + +def test_topas_version_kwarg_in_range_calls_engine(tmp_path, monkeypatch): + """topas_version=7 (in range) calls run_topas_recipe.""" + from powderline.topas import engine + + fake_run = Mock(return_value={}) + monkeypatch.setattr(engine, "run_topas_recipe", fake_run) + + topas_gateway.run(_lab6_recipe(), str(tmp_path), topas_version=7, validate_only=False) + assert fake_run.called + + +def test_topas_config_version_out_of_range_raises(tmp_path, monkeypatch): + """Config {"version": 6} with topas_version=None raises EngineVersionError.""" + from powderline.topas import runner, engine + + monkeypatch.setattr(runner, "_read_topas_config", lambda: {"version": 6}) + fake_run = Mock() + monkeypatch.setattr(engine, "run_topas_recipe", fake_run) + + with pytest.raises(EngineVersionError): + topas_gateway.run(_lab6_recipe(), str(tmp_path), validate_only=False) + assert not fake_run.called + + +def test_topas_allow_unsupported_bypasses_version_check(tmp_path, monkeypatch): + """allow_unsupported_engine_version=True bypasses the version check.""" + from powderline.topas import engine + + fake_run = Mock(return_value={}) + monkeypatch.setattr(engine, "run_topas_recipe", fake_run) + + topas_gateway.run( + _lab6_recipe(), + str(tmp_path), + topas_version=6, + allow_unsupported_engine_version=True, + validate_only=False, + ) + assert fake_run.called + + +def test_topas_validate_only_skips_version_check(tmp_path, monkeypatch): + """With validate_only=True, the version check is skipped even with topas_version=6.""" + from powderline.topas import engine + + fake_run = Mock(return_value={}) + monkeypatch.setattr(engine, "run_topas_recipe", fake_run) + + topas_gateway.run(_lab6_recipe(), str(tmp_path), topas_version=6, validate_only=True) + assert fake_run.called + + +# --- easydiffraction version checking --- + + +def test_easydiff_version_out_of_range_raises(tmp_path, monkeypatch): + """When installed_engine_version() returns '0.21.0', run() raises EngineVersionError.""" + monkeypatch.setattr(easydiff_gateway, "installed_engine_version", lambda: "0.21.0") + from powderline.easydiff import engine + + fake_run = Mock() + monkeypatch.setattr(engine, "run_easydiffraction_recipe", fake_run) + + with pytest.raises(EngineVersionError): + easydiff_gateway.run(_lab6_recipe(), str(tmp_path), validate_only=False) + assert not fake_run.called + + +def test_easydiff_version_in_range_calls_engine(tmp_path, monkeypatch): + """When installed_engine_version() returns '0.20.1', run() calls the engine.""" + monkeypatch.setattr(easydiff_gateway, "installed_engine_version", lambda: "0.20.1") + from powderline.easydiff import engine + + fake_run = Mock(return_value={}) + monkeypatch.setattr(engine, "run_easydiffraction_recipe", fake_run) + + easydiff_gateway.run(_lab6_recipe(), str(tmp_path), validate_only=False) + assert fake_run.called + + +@pytest.mark.skipif( + importlib.util.find_spec("easydiffraction") is not None, + reason="easydiffraction is installed", +) +def test_easydiff_not_installed_raises_engine_not_available_error(tmp_path): + """When easydiffraction is not installed, run() raises EngineNotAvailableError.""" + with pytest.raises(EngineNotAvailableError) as exc_info: + easydiff_gateway.run(_lab6_recipe(), str(tmp_path), validate_only=False) + assert isinstance(exc_info.value, ImportError) + assert "pixi install -e easydiff" in str(exc_info.value) + + +@pytest.mark.skipif( + importlib.util.find_spec("easydiffraction") is not None, + reason="easydiffraction is installed", +) +def test_easydiff_not_installed_validate_only_does_not_raise(tmp_path, monkeypatch): + """When easydiffraction is not installed, validate_only=True does not raise.""" + from powderline.easydiff import engine + + fake_run = Mock(return_value={}) + monkeypatch.setattr(engine, "run_easydiffraction_recipe", fake_run) + + # validate_only should not trigger the import that raises EngineNotAvailableError + easydiff_gateway.run(_lab6_recipe(), str(tmp_path), validate_only=True) + assert fake_run.called + + +@pytest.mark.skipif( + importlib.util.find_spec("easydiffraction") is None, + reason="easydiffraction not installed", +) +def test_easydiff_installed_version_satisfies_spec(): + """When easydiffraction is installed, its version satisfies ENGINE_VERSION_SPEC. + + easydiffraction 0.20.1 has no ``__version__``; its own runtime version source + is ``utils.package_version`` (distribution metadata), which must agree. + """ + from easydiffraction.utils.utils import package_version + + version = easydiff_gateway.installed_engine_version() + assert version == package_version("easydiffraction") + spec = SpecifierSet(easydiff_gateway.ENGINE_VERSION_SPEC) + assert version in spec + + +# --- Dispatcher routing --- + + +def test_dispatcher_routes_through_registry_get(tmp_path, monkeypatch): + """powderline.engine.run(..., engine=X) routes through registry.get(X).run(...).""" + from powderline import engine as dispatcher_module + + # Register fake gateways that record calls + calls = {} + + def make_fake_gateway(name): + def fake_run(recipe, output_dir, **kwargs): + calls[name] = kwargs + return {} + + return registry.Gateway( + name=name, + capabilities=lambda: {"name": name, "workflows": [], "engine_version_spec": ">=1"}, + validate=lambda recipe, **opts: recipe, + run=fake_run, + ) + + for name in ["gsasii", "topas", "easydiffraction"]: + registry.register_gateway(name, lambda n=name: make_fake_gateway(n)) + + # Test gsasii routing + dispatcher_module.run( + _lab6_recipe(), + str(tmp_path), + engine="gsasii", + verbose=True, + validate_only=False, + execution_mode="subprocess", + ) + assert "gsasii" in calls + assert calls["gsasii"]["verbose"] is True + assert calls["gsasii"]["validate_only"] is False + assert calls["gsasii"]["execution_mode"] == "subprocess" + + # Test topas routing + calls.clear() + dispatcher_module.run( + _lab6_recipe(), + str(tmp_path), + engine="topas", + verbose=False, + validate_only=True, + topas_dir="/fake", + topas_version=7, + ) + assert "topas" in calls + assert calls["topas"]["verbose"] is False + assert calls["topas"]["validate_only"] is True + assert calls["topas"]["topas_dir"] == "/fake" + assert calls["topas"]["topas_version"] == 7 + + # Test easydiffraction routing + calls.clear() + dispatcher_module.run( + _lab6_recipe(), + str(tmp_path), + engine="easydiffraction", + verbose=True, + validate_only=False, + ) + assert "easydiffraction" in calls + assert calls["easydiffraction"]["verbose"] is True + assert calls["easydiffraction"]["validate_only"] is False + + # Unregister the fake gateways + for name in ["gsasii", "topas", "easydiffraction"]: + registry.unregister_gateway(name) + + +def test_dispatcher_unknown_engine_raises_value_error(): + """powderline.engine.run(..., engine='bogus') raises ValueError.""" + from powderline import engine as dispatcher_module + + with pytest.raises(ValueError, match="unknown engine"): + dispatcher_module.run(_lab6_recipe(), "/tmp", engine="bogus") + + +# --- Import-block guarantee --- + + +def test_import_block_schema_layer_and_validate(): + """Schema-layer imports (registry, gateways, capabilities, validate) work with engines blocked.""" + recipe_path = str(EXAMPLES / "example_LaB6" / "input.json") + easydiff_recipe_path = str(EXAMPLES / "example_LaB6_easydiff" / "input.json") + script = f""" +import sys, importlib.abc, json +from pathlib import Path + +# Block GSASII and easydiffraction imports +class _Block(importlib.abc.MetaPathFinder): + def find_spec(self, name, path, target=None): + if name in ("GSASII", "easydiffraction") or name.startswith(("GSASII.", "easydiffraction.")): + raise ImportError(f"{{name}} blocked for import-block test") + return None + +sys.meta_path.insert(0, _Block()) + +# Import the engine-free core + schema-layer modules +import powderline +import powderline.registry +import powderline.exceptions +import powderline.fitstats +import powderline.reports +import powderline.symmetry +import powderline.gateways.gsasii +import powderline.gateways.gsasii.gateway +import powderline.gateways.gsasii.validation +import powderline.topas.gateway +import powderline.easydiff.gateway +from powderline import registry + +recipes = {{ + "gsasii": json.loads(Path(r'{recipe_path}').read_text(encoding="utf-8")), + "topas": json.loads(Path(r'{recipe_path}').read_text(encoding="utf-8")), + # the GSAS-II LaB6 recipe refines Z, which easydiffraction rejects + "easydiffraction": json.loads(Path(r'{easydiff_recipe_path}').read_text(encoding="utf-8")), +}} + +# The schema-only consumer path, through the registry (entry points) +for name, recipe in recipes.items(): + gw = registry.get(name) + caps = gw.capabilities() + assert caps["name"] == name and "engine_version_spec" in caps + assert type(gw.validate(recipe)).__name__ == "RecipeModel" + +# Assert GSASII and easydiffraction are NOT in sys.modules +assert "GSASII" not in sys.modules, "GSASII was imported despite the block" +assert "easydiffraction" not in sys.modules, "easydiffraction was imported despite the block" + +print("OK") +""" + proc = run_subprocess_utf8([sys.executable, "-c", script], capture_output=True, text=True) + assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + assert "OK" in proc.stdout + + +def test_import_block_run_raises_engine_not_available_error(tmp_path): + """With GSASII blocked, gsasii gateway run() raises EngineNotAvailableError.""" + recipe_path = str(EXAMPLES / "example_LaB6" / "input.json") + output_path = str(tmp_path) + script = f""" +import sys, importlib.abc, json +from pathlib import Path + +# Block GSASII import +class _Block(importlib.abc.MetaPathFinder): + def find_spec(self, name, path, target=None): + if name == "GSASII" or name.startswith("GSASII."): + raise ImportError("GSASII blocked for import-block test") + return None + +sys.meta_path.insert(0, _Block()) + +from powderline.gateways.gsasii import gateway as gsasii_gw +from powderline.exceptions import EngineNotAvailableError + +recipe = json.loads(Path(r'{recipe_path}').read_text()) + +try: + gsasii_gw.run(recipe, r'{output_path}', validate_only=False) + print("ERROR: should have raised EngineNotAvailableError") + sys.exit(1) +except EngineNotAvailableError as e: + assert isinstance(e, ImportError) + print("OK") +""" + proc = run_subprocess_utf8([sys.executable, "-c", script], capture_output=True, text=True) + assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + assert "OK" in proc.stdout diff --git a/tests/test_registry.py b/tests/test_registry.py new file mode 100644 index 0000000..657fc44 --- /dev/null +++ b/tests/test_registry.py @@ -0,0 +1,284 @@ +"""Tests for the gateway registry: entry-point discovery, explicit registration, version checks. + +Key invariants (A17/A7/A54): +- Discovery is lazy: available() lists entry points without loading them; get() loads on first use. +- register_gateway() takes precedence over an entry point of the same name. +- A factory returning a non-Gateway raises TypeError. +- check_engine_version enforces PEP 440 specs; allow_unsupported=True bypasses. +- Entry points are only present after re-installing the editable package (pixi reinstall powderline). +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from unittest.mock import Mock + +import pytest +from packaging.specifiers import SpecifierSet + +from powderline import registry +from powderline.exceptions import EngineVersionError, GatewayNotInstalledError +from subprocess_utils import run_subprocess_utf8 + +REPO = Path(__file__).resolve().parent.parent + + +@pytest.fixture(autouse=True) +def _reset_registry_caches(monkeypatch): + """Snapshot and restore the module-level registry caches before/after each test.""" + original_explicit = registry._explicit.copy() + original_instances = registry._instances.copy() + original_entry_points = registry._entry_points + + yield + + # Restore the original state + registry._explicit.clear() + registry._explicit.update(original_explicit) + registry._instances.clear() + registry._instances.update(original_instances) + # Reset entry points to force re-discovery + monkeypatch.setattr(registry, "_entry_points", original_entry_points) + + +def _fake_gateway(name="fake"): + """Build a minimal Gateway for testing.""" + return registry.Gateway( + name=name, + capabilities=lambda: {"name": name, "workflows": [], "engine_version_spec": ">=1"}, + validate=lambda recipe, **opts: recipe, + run=lambda recipe, output_dir, **opts: {}, + ) + + +def _fake_entry_point(name, gateway): + """Build a fake entry point that tracks load() calls.""" + ep = Mock() + ep.name = name + ep.load_called = False + original_gateway = gateway + + def load(): + ep.load_called = True + return lambda: original_gateway + + ep.load = load + return ep + + +# --- register_gateway escape hatch --- + + +def test_register_gateway_makes_get_return_it(): + factory = lambda: _fake_gateway("test_gw") + registry.register_gateway("test_gw", factory) + assert registry.get("test_gw").name == "test_gw" + + +def test_register_gateway_is_cached(monkeypatch): + call_count = 0 + + def factory(): + nonlocal call_count + call_count += 1 + return _fake_gateway("cached") + + registry.register_gateway("cached", factory) + registry.get("cached") + registry.get("cached") + assert call_count == 1, "factory should be called exactly once (cached)" + + +def test_explicit_registration_takes_precedence_over_entry_point(monkeypatch): + """An explicit registration shadows an entry point of the same name.""" + gw_from_ep = _fake_gateway("from_ep") + gw_explicit = _fake_gateway("from_explicit") + ep = _fake_entry_point("shadowed", gw_from_ep) + monkeypatch.setattr(registry, "_entry_points", {"shadowed": ep}) + + registry.register_gateway("shadowed", lambda: gw_explicit) + assert registry.get("shadowed").name == "from_explicit" + assert not ep.load_called, "entry point should not be loaded when shadowed" + + +def test_unregister_gateway_removes_explicit(): + registry.register_gateway("to_remove", lambda: _fake_gateway("to_remove")) + registry.get("to_remove") # cache it + registry.unregister_gateway("to_remove") + with pytest.raises(GatewayNotInstalledError, match="to_remove"): + registry.get("to_remove") + + +def test_available_lists_explicit_and_entry_point_names_sorted(monkeypatch): + registry.register_gateway("explicit_a", lambda: _fake_gateway("explicit_a")) + registry.register_gateway("explicit_z", lambda: _fake_gateway("explicit_z")) + ep1 = _fake_entry_point("ep_b", _fake_gateway("ep_b")) + ep2 = _fake_entry_point("ep_y", _fake_gateway("ep_y")) + monkeypatch.setattr(registry, "_entry_points", {"ep_b": ep1, "ep_y": ep2}) + + names = registry.available() + assert names == ["ep_b", "ep_y", "explicit_a", "explicit_z"] + assert not ep1.load_called, "available() should not load entry points" + assert not ep2.load_called, "available() should not load entry points" + + +# --- Lazy loading --- + + +def test_available_does_not_load_entry_points(monkeypatch): + ep = _fake_entry_point("lazy_test", _fake_gateway("lazy_test")) + monkeypatch.setattr(registry, "_entry_points", {"lazy_test": ep}) + registry.available() + assert not ep.load_called, "available() should not call ep.load()" + + +def test_get_loads_entry_point_exactly_once(monkeypatch): + load_count = 0 + gw = _fake_gateway("lazy_gw") + + def load(): + nonlocal load_count + load_count += 1 + return lambda: gw + + ep = Mock() + ep.name = "lazy_gw" + ep.load = load + monkeypatch.setattr(registry, "_entry_points", {"lazy_gw": ep}) + + registry.get("lazy_gw") + registry.get("lazy_gw") + assert load_count == 1, "entry point load() should be called exactly once" + + +# --- get() of unregistered name --- + + +def test_get_unregistered_name_raises_gateway_not_installed_error(monkeypatch): + monkeypatch.setattr(registry, "_entry_points", {"known": Mock()}) + with pytest.raises(GatewayNotInstalledError) as exc_info: + registry.get("unknown") + assert exc_info.value.gateway == "unknown" + msg = str(exc_info.value) + assert "unknown" in msg + assert "pip install" in msg or "register_gateway" in msg + + +def test_zero_entry_points_message_mentions_stale_metadata(monkeypatch): + """When no entry points are found at all, message says re-install.""" + monkeypatch.setattr(registry, "_entry_points", {}) + with pytest.raises(GatewayNotInstalledError) as exc_info: + registry.get("anything") + msg = str(exc_info.value) + assert "no 'powderline.gateways' entry points were found" in msg + assert "pixi reinstall powderline" in msg + + +# --- factory returning non-Gateway --- + + +def test_factory_returning_non_gateway_raises_type_error(monkeypatch): + registry.register_gateway("bad", lambda: "not a Gateway") + with pytest.raises(TypeError, match="expected powderline.registry.Gateway"): + registry.get("bad") + + +# --- check_engine_version --- + + +def test_check_engine_version_in_range_passes(): + registry.check_engine_version( + gateway="test", engine="TestEngine", installed="1.2.3", spec=">=1,<2" + ) + + +def test_check_engine_version_out_of_range_raises(): + with pytest.raises(EngineVersionError) as exc_info: + registry.check_engine_version( + gateway="test", engine="TestEngine", installed="0.9.0", spec=">=1,<2" + ) + assert exc_info.value.installed == "0.9.0" + assert exc_info.value.supported == ">=1,<2" + + +def test_check_engine_version_invalid_version_raises(): + with pytest.raises(EngineVersionError): + registry.check_engine_version( + gateway="test", engine="TestEngine", installed="not-a-version", spec=">=1,<2" + ) + + +def test_check_engine_version_allow_unsupported_bypasses(): + registry.check_engine_version( + gateway="test", + engine="TestEngine", + installed="0.9.0", + spec=">=1,<2", + allow_unsupported=True, + ) + + +def test_check_engine_version_topas_bare_int(): + """TOPAS-style bare int '7' against '>=7,<8' passes.""" + registry.check_engine_version( + gateway="topas", engine="TOPAS", installed="7", spec=">=7,<8" + ) + + +def test_check_engine_version_topas_bare_int_out_of_range(): + """TOPAS-style bare int '6' against '>=7,<8' fails.""" + with pytest.raises(EngineVersionError): + registry.check_engine_version( + gateway="topas", engine="TOPAS", installed="6", spec=">=7,<8" + ) + + +# --- REAL entry points (requires re-install) --- + + +def test_real_entry_points_available(monkeypatch): + """With caches reset, available() includes the three built-in gateways.""" + # Reset to force discovery + monkeypatch.setattr(registry, "_entry_points", None) + monkeypatch.setattr(registry, "_instances", {}) + names = registry.available() + # Expected to fail until `pixi reinstall powderline` is run + assert {"gsasii", "topas", "easydiffraction"} <= set(names) + + +def test_real_entry_points_get_returns_gateway(monkeypatch): + """With caches reset, get(name) returns a Gateway for each built-in.""" + monkeypatch.setattr(registry, "_entry_points", None) + monkeypatch.setattr(registry, "_instances", {}) + for name in ["gsasii", "topas", "easydiffraction"]: + gw = registry.get(name) + assert gw.name == name + assert isinstance(gw, registry.Gateway) + + +# --- PYTHONPATH=src mode --- + + +def test_pythonpath_src_mode_sees_entry_points(): + """Subprocess with PYTHONPATH=src sees the three built-in entry points.""" + import os + + script = """ +import sys +import os +from importlib.metadata import entry_points + +sys.path.insert(0, os.environ.get("PYTHONPATH", "")) +eps = entry_points(group="powderline.gateways") +names = sorted(ep.name for ep in eps) +print(",".join(names)) +""" + env = {"PYTHONPATH": str(REPO / "src")} + proc = run_subprocess_utf8( + [sys.executable, "-c", script], capture_output=True, text=True, env={**os.environ, **env} + ) + assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + names = proc.stdout.strip().split(",") + # Expected to fail until re-install + assert set(names) >= {"easydiffraction", "gsasii", "topas"} From 4e1bc1b4fce8b03193dc4155b2718b873597b4c5 Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Tue, 29 Sep 2026 13:53:04 -0400 Subject: [PATCH 07/10] fix(core): route every symmetry raise through one lazy error helper (G1 KI-13) The A56 relocation re-imported TopasTranslationError lazily in only two of the four raising functions; cell_constraints and site_dof would have raised NameError on their (unreachable with real gemmi groups) fallthrough paths. All six raise sites now use _translation_error(), the module's single reference to a gateway, so re/03's switch to a core error is one line (A57). Tests force all three defensive paths; an undefined-name probe over every module changed since v0.1.1 reports none. Co-Authored-By: Claude Opus 5.5 --- src/powderline/symmetry.py | 34 ++++++++++++++++++++-------------- tests/test_topas_symmetry.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/powderline/symmetry.py b/src/powderline/symmetry.py index e38aedf..09db7e7 100644 --- a/src/powderline/symmetry.py +++ b/src/powderline/symmetry.py @@ -84,6 +84,20 @@ def classification(self, axis: str) -> str: return self.axes[_AXIS_LETTERS.index(axis)] +def _translation_error(message: str) -> Exception: + """Build the error raised for unsupported symmetry input (A56, interim). + + Returns ``powderline.topas.errors.TopasTranslationError``, the type callers + catch today. Imported here, lazily, because importing ``powderline.topas`` at + module top is circular (topas/__init__ -> writer -> symmetry). This is the + module's only reference to a gateway; re/03 replaces it with a core error + that each gateway translates. + """ + from powderline.topas.errors import TopasTranslationError + + return TopasTranslationError(message) + + # --- space-group resolution ------------------------------------------------- @@ -96,25 +110,21 @@ def resolve_space_group(space_group: str) -> gemmi.SpaceGroup: passed without an explicit ``:1``/``:2`` selector also error rather than guess an origin (plan §5(6); demo groups are single-origin). """ - # Lazy: importing powderline.topas at module top is circular (topas/__init__ - # -> writer -> symmetry). Interim until re/03 raises a core error here (A56). - from powderline.topas.errors import TopasTranslationError - raw = str(space_group) sg = gemmi.find_spacegroup_by_name(raw) if sg is None: sg = gemmi.find_spacegroup_by_name(raw.replace(" ", "")) if sg is None: - raise TopasTranslationError( + raise _translation_error( f"unrecognized space-group symbol {space_group!r} (gemmi could not resolve it)" ) if sg.ext == "R": - raise TopasTranslationError( + raise _translation_error( f"rhombohedral ':R' setting not supported for {space_group!r}; " "use the hexagonal (:H) setting" ) if sg.ext in ("1", "2") and ":" not in raw: - raise TopasTranslationError( + raise _translation_error( f"space group {space_group!r} has two origin choices; specify one " "explicitly (e.g. append ':2' for the GSAS-II origin-2 convention)" ) @@ -141,10 +151,6 @@ def _monoclinic_unique_axis(ops: list[tuple[np.ndarray, np.ndarray]]) -> str: The single proper 2-fold (det +1, trace -1) fixes the unique axis; its invariant direction maps to the dominant cell axis a/b/c. """ - # Lazy: importing powderline.topas at module top is circular (topas/__init__ - # -> writer -> symmetry). Interim until re/03 raises a core error here (A56). - from powderline.topas.errors import TopasTranslationError - for R, _t in ops: if abs(np.linalg.det(R) - 1.0) < _MAT_TOL and abs(np.trace(R) + 1.0) < _MAT_TOL: evals, evecs = np.linalg.eig(R) @@ -152,7 +158,7 @@ def _monoclinic_unique_axis(ops: list[tuple[np.ndarray, np.ndarray]]) -> str: if abs(evals[i].real - 1.0) < _MAT_TOL and abs(evals[i].imag) < _MAT_TOL: direction = np.abs(evecs[:, i].real) return _LENGTHS[int(np.argmax(direction))] - raise TopasTranslationError( + raise _translation_error( "could not determine the monoclinic unique axis (no proper 2-fold found)" ) @@ -187,7 +193,7 @@ def cell_constraints(space_group: str) -> CellRules: if system == "triclinic": return CellRules(system, singles, (), _ANGLES) - raise TopasTranslationError( + raise _translation_error( f"unsupported crystal system {system!r} for space group {space_group!r}" ) @@ -229,7 +235,7 @@ def site_dof(space_group: str, xyz) -> SiteDof: stab_rots = [R for (R, t) in ops if np.all(np.abs(_wrap_symmetric(R @ x + t - x)) < _POS_TOL)] if not stab_rots: # pragma: no cover - identity is always in the group - raise TopasTranslationError("empty stabilizer (should be impossible)") + raise _translation_error("empty stabilizer (should be impossible)") projector = sum(stab_rots) / len(stab_rots) diff --git a/tests/test_topas_symmetry.py b/tests/test_topas_symmetry.py index 27295c1..5288a84 100644 --- a/tests/test_topas_symmetry.py +++ b/tests/test_topas_symmetry.py @@ -168,3 +168,35 @@ def test_adp_dof_c2m_unique_b_off_diagonals_fixed(): def test_adp_dof_rejects_bad_shape(): with pytest.raises(ValueError): sym.adp_dof("P m -3 m", (0.0, 0.0)) + + +# --- defensive raise paths (unreachable with real gemmi groups; KI-13) ------- + + +class _UnknownSystem: + """Stand-in space group whose crystal system no rule handles.""" + + def crystal_system_str(self): + return "not-a-system" + + +def test_unsupported_crystal_system_raises_translation_error(monkeypatch): + monkeypatch.setattr(sym, "resolve_space_group", lambda _sg: _UnknownSystem()) + with pytest.raises(TopasTranslationError, match="unsupported crystal system"): + sym.cell_constraints("P1") + + +def test_empty_stabilizer_raises_translation_error(monkeypatch): + import numpy as np + + # A single translation-only op fixes no point, so the stabiliser is empty. + monkeypatch.setattr(sym, "_expanded_ops", lambda _sg: [(np.eye(3), np.full(3, 0.5))]) + with pytest.raises(TopasTranslationError, match="empty stabilizer"): + sym.site_dof("P1", [0.1, 0.2, 0.3]) + + +def test_monoclinic_unique_axis_failure_raises_translation_error(): + import numpy as np + + with pytest.raises(TopasTranslationError, match="monoclinic unique axis"): + sym._monoclinic_unique_axis([(np.eye(3), np.zeros(3))]) From 04b136861a7d8ad80fa56c6551b060442c863018 Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Tue, 29 Sep 2026 13:53:04 -0400 Subject: [PATCH 08/10] fix(core): fitstats rejects non-finite weights inside the fit range (G1 nit) NaN > 0 is False, so a NaN weight was silently dropped from N instead of raising as documented. Weights are now checked over the whole fit range before the w > 0 filter. Co-Authored-By: Claude Opus 5.5 --- src/powderline/fitstats.py | 6 +++++- tests/test_fitstats.py | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/powderline/fitstats.py b/src/powderline/fitstats.py index 7b4938b..e175888 100644 --- a/src/powderline/fitstats.py +++ b/src/powderline/fitstats.py @@ -78,12 +78,16 @@ def compute_fit_statistics(yobs, ycalc, weights, n_params: int, if int(n_params) != n_params or n_params < 0: raise ValueError(f"n_params must be a non-negative integer, got {n_params!r}") n_params = int(n_params) + # Check weights over the whole fit range *before* the w > 0 filter: NaN > 0 + # is False, so a NaN weight would otherwise be silently dropped from N. + if not np.all(np.isfinite(weights[mask])): + raise ValueError("yobs, ycalc and weights must be finite inside the fit range") if np.any(weights[mask] < 0): raise ValueError("weights must be >= 0") included = mask & (weights > 0) yo, yc, w = yobs[included], ycalc[included], weights[included] - if not (np.all(np.isfinite(yo)) and np.all(np.isfinite(yc)) and np.all(np.isfinite(w))): + if not (np.all(np.isfinite(yo)) and np.all(np.isfinite(yc))): raise ValueError("yobs, ycalc and weights must be finite inside the fit range") n_points = int(included.sum()) diff --git a/tests/test_fitstats.py b/tests/test_fitstats.py index c2bc674..4247031 100644 --- a/tests/test_fitstats.py +++ b/tests/test_fitstats.py @@ -61,6 +61,8 @@ def test_undefined_values_are_nan(): (dict(yobs=[1, 2], ycalc=[1, 2], weights=[1, 1], n_params=-1), "non-negative integer"), (dict(yobs=[1, 2], ycalc=[1, 2], weights=[1, 1], n_params=1.5), "non-negative integer"), (dict(yobs=[1, np.nan], ycalc=[1, 2], weights=[1, 1], n_params=0), "finite"), + (dict(yobs=[1, 2, 3], ycalc=[1, 2, 3], weights=[1, np.nan, 1], n_params=0), "finite"), + (dict(yobs=[1, 2], ycalc=[1, 2], weights=[1, np.inf], n_params=0), "finite"), (dict(yobs=[1, 2], ycalc=[1, 2], weights=[1, 1], n_params=0, mask=[True]), "mask"), ]) def test_invalid_input_raises(kwargs, match): From 33f510c053271eb9096ba166a4332c83a5a8096d Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Tue, 29 Sep 2026 13:53:05 -0400 Subject: [PATCH 09/10] fix(gateways): uniform validate(recipe, *, verbose=False) contract; test hygiene (G1 nits) - Every gateway's validate() takes the same explicit keywords (A59); topas and easydiffraction previously took none, so registry.get(n).validate(r, verbose=True) raised TypeError for them. Tests assert the signature and the call through the registry for all three. - The dispatcher-routing test restores the registry via a fixture, even on failure (it cleaned up only on success). - Recipe reads in test_gateways pin encoding="utf-8" (Windows, G3). Co-Authored-By: Claude Opus 5.5 --- src/powderline/easydiff/gateway.py | 8 +++-- src/powderline/gateways/gsasii/gateway.py | 2 +- src/powderline/registry.py | 7 ++-- src/powderline/topas/gateway.py | 8 +++-- tests/test_gateways.py | 43 +++++++++++++++++++---- 5 files changed, 53 insertions(+), 15 deletions(-) diff --git a/src/powderline/easydiff/gateway.py b/src/powderline/easydiff/gateway.py index dae0738..57728ae 100644 --- a/src/powderline/easydiff/gateway.py +++ b/src/powderline/easydiff/gateway.py @@ -31,8 +31,12 @@ def capabilities() -> dict: return {"name": NAME, "workflows": WORKFLOWS, "engine_version_spec": ENGINE_VERSION_SPEC} -def validate(recipe): - """Validate a recipe (dict or ``RecipeModel``); return the ``RecipeModel`` (A50).""" +def validate(recipe, *, verbose: bool = False): + """Validate a recipe (dict or ``RecipeModel``); return the ``RecipeModel`` (A50, A59). + + ``verbose`` is part of the uniform gateway contract; this gateway prints + nothing extra today. + """ from powderline.easydiff.policy import check_unsupported model = recipe if isinstance(recipe, RecipeModel) else RecipeModel.model_validate(recipe) diff --git a/src/powderline/gateways/gsasii/gateway.py b/src/powderline/gateways/gsasii/gateway.py index 1cab87b..5ea97a1 100644 --- a/src/powderline/gateways/gsasii/gateway.py +++ b/src/powderline/gateways/gsasii/gateway.py @@ -32,7 +32,7 @@ def capabilities() -> dict: def validate(recipe, *, verbose: bool = False): - """Validate a recipe (dict or ``RecipeModel``); return the ``RecipeModel`` (A50).""" + """Validate a recipe (dict or ``RecipeModel``); return the ``RecipeModel`` (A50, A59).""" return _validate_recipe(recipe, verbose=verbose) diff --git a/src/powderline/registry.py b/src/powderline/registry.py index d2163d8..732b193 100644 --- a/src/powderline/registry.py +++ b/src/powderline/registry.py @@ -41,9 +41,10 @@ class Gateway: - ``name``: registry name (``"gsasii"``, ``"topas"``, ``"easydiffraction"``). - ``capabilities()``: dict with at least ``name``, ``workflows`` and ``engine_version_spec`` (a PEP 440 specifier string, A41). - - ``validate(recipe, **opts)``: schema layer, engine-free. Takes a recipe - dict or an already-validated model, raises on an invalid recipe, returns - the validated model (A50). + - ``validate(recipe, *, verbose=False)``: schema layer, engine-free. Takes a + recipe dict or an already-validated model, raises on an invalid recipe, + returns the validated model (A50). The same explicit keywords on every + gateway (A59); new options are added to the contract deliberately. - ``run(recipe, output_dir, **opts)``: runtime layer; imports the engine lazily; returns the standardized result dict. """ diff --git a/src/powderline/topas/gateway.py b/src/powderline/topas/gateway.py index 9beb886..b782426 100644 --- a/src/powderline/topas/gateway.py +++ b/src/powderline/topas/gateway.py @@ -30,8 +30,12 @@ def capabilities() -> dict: return {"name": NAME, "workflows": WORKFLOWS, "engine_version_spec": ENGINE_VERSION_SPEC} -def validate(recipe): - """Validate a recipe (dict or ``RecipeModel``); return the ``RecipeModel`` (A50).""" +def validate(recipe, *, verbose: bool = False): + """Validate a recipe (dict or ``RecipeModel``); return the ``RecipeModel`` (A50, A59). + + ``verbose`` is part of the uniform gateway contract; this gateway prints + nothing extra today. + """ from powderline.topas.writer import render_topas model = recipe if isinstance(recipe, RecipeModel) else RecipeModel.model_validate(recipe) diff --git a/tests/test_gateways.py b/tests/test_gateways.py index f7cd33b..a1e8cc8 100644 --- a/tests/test_gateways.py +++ b/tests/test_gateways.py @@ -33,7 +33,19 @@ def _lab6_recipe(): """Load the LaB6 example recipe as a dict.""" - return json.loads((EXAMPLES / "example_LaB6" / "input.json").read_text()) + return json.loads((EXAMPLES / "example_LaB6" / "input.json").read_text(encoding="utf-8")) + + +@pytest.fixture +def isolated_registry(): + """Snapshot the registry caches; restore them even if the test fails.""" + saved = (registry._explicit.copy(), registry._instances.copy(), registry._entry_points) + yield registry + registry._explicit.clear() + registry._explicit.update(saved[0]) + registry._instances.clear() + registry._instances.update(saved[1]) + registry._entry_points = saved[2] # --- ENGINE_VERSION_SPEC consistency --- @@ -351,7 +363,7 @@ def test_easydiff_installed_version_satisfies_spec(): # --- Dispatcher routing --- -def test_dispatcher_routes_through_registry_get(tmp_path, monkeypatch): +def test_dispatcher_routes_through_registry_get(tmp_path, monkeypatch, isolated_registry): """powderline.engine.run(..., engine=X) routes through registry.get(X).run(...).""" from powderline import engine as dispatcher_module @@ -417,10 +429,6 @@ def fake_run(recipe, output_dir, **kwargs): assert calls["easydiffraction"]["verbose"] is True assert calls["easydiffraction"]["validate_only"] is False - # Unregister the fake gateways - for name in ["gsasii", "topas", "easydiffraction"]: - registry.unregister_gateway(name) - def test_dispatcher_unknown_engine_raises_value_error(): """powderline.engine.run(..., engine='bogus') raises ValueError.""" @@ -509,7 +517,7 @@ def find_spec(self, name, path, target=None): from powderline.gateways.gsasii import gateway as gsasii_gw from powderline.exceptions import EngineNotAvailableError -recipe = json.loads(Path(r'{recipe_path}').read_text()) +recipe = json.loads(Path(r'{recipe_path}').read_text(encoding="utf-8")) try: gsasii_gw.run(recipe, r'{output_path}', validate_only=False) @@ -522,3 +530,24 @@ def find_spec(self, name, path, target=None): proc = run_subprocess_utf8([sys.executable, "-c", script], capture_output=True, text=True) assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" assert "OK" in proc.stdout + + +# --- Uniform validate() contract (A59) --- + + +@pytest.mark.parametrize("gateway_module", [gsasii_gateway, topas_gateway, easydiff_gateway], + ids=["gsasii", "topas", "easydiffraction"]) +def test_validate_signature_is_uniform(gateway_module): + import inspect + + params = inspect.signature(gateway_module.validate).parameters + assert list(params) == ["recipe", "verbose"] + assert params["verbose"].kind is inspect.Parameter.KEYWORD_ONLY + assert params["verbose"].default is False + + +@pytest.mark.parametrize("name", ["gsasii", "topas", "easydiffraction"]) +def test_validate_accepts_verbose_through_the_registry(name): + example = "example_LaB6_easydiff" if name == "easydiffraction" else "example_LaB6" + recipe = json.loads((EXAMPLES / example / "input.json").read_text(encoding="utf-8")) + assert isinstance(registry.get(name).validate(recipe, verbose=True), schema.RecipeModel) From a84fd39028c65c1bbc1174f68a875824e24668b1 Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Tue, 29 Sep 2026 13:53:05 -0400 Subject: [PATCH 10/10] docs(known_issues): KI-13 - engine dispatch depends on entry-point metadata (watch) Filed by the G1 review (as KI-14; renumbered because the review's KI-13, a re/02 regression, was fixed before merge and never entered the register). Decision A58: entry points stay the only discovery path; re/08 adds install-health guidance, re/10 documents the re-install step. Co-Authored-By: Claude Opus 5.5 --- docs/known_issues.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/known_issues.md b/docs/known_issues.md index 08e103e..12b57b2 100644 --- a/docs/known_issues.md +++ b/docs/known_issues.md @@ -356,3 +356,35 @@ supported topology. Needs protocol versioning for already-running servers. **Revisit.** Next branch touching the server protocol, or when the HPC deployment work starts. + +--- + +## KI-13 — Engine dispatch depends on install-time entry-point metadata `watch · minor` + +**What.** Since re/02, `powderline.run()` resolves every engine through the +gateway registry. That includes the default `engine="gsasii"` and the in-repo +gateways. The registry reads the `powderline.gateways` entry points from the +installed distribution's metadata. If a checkout's editable install predates +the `[project.entry-points]` table, is half-installed, or powderline is +imported from source (`PYTHONPATH=src`) with no dist-info at all, then every +`run()` call fails with `GatewayNotInstalledError`. Before re/02 the +dispatcher imported the backends directly and did not depend on install +metadata. The error message is clear and names the fix. However, each existing +dev environment has to be re-installed once (`pixi reinstall powderline`). +During re/02 the maintainer's `easydiff` env was left half-installed with no +error shown, and it failed this way. + +**Evidence.** `src/powderline/engine.py:53` (`registry.get(...)` for all +engines); `src/powderline/registry.py` `get` / `_not_installed_message`; +`CLAUDE.md` (the `pixi reinstall powderline` note); the execution-log re/02 +entry (env notes). + +**Decision.** Keep entry points as the **only** discovery path (A17, A58): the +in-repo gateways use exactly the plugin mechanism a third-party engine will +use, and a hard-coded fallback would add a second discovery path and hide a +stale install. The registry's error already names the fix. re/08 adds +install-health guidance (and possibly a check command); re/10 documents the +re-install step for contributors. + +**Revisit.** re/08 (entry points finalized) and re/10 (migration guide / dev +docs).