From 47ec06a66634c32ef3f813a08f044ac8fabfacc3 Mon Sep 17 00:00:00 2001 From: Delun Gong Date: Wed, 12 Aug 2026 20:22:23 +0800 Subject: [PATCH 1/2] feat: improve GUI layout and Excel analysis long tables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scrollable form columns, resizable activity split, pinned run buttons. Analysis-first Peaks/推荐峰表 columns with freeze/filter, clearer 使用说明 (rank 1=strongest, 2θ=2×θ), plus related pipeline/test hardening. Verified: pytest 115 passed. --- CHANGELOG.md | 11 + docs/API.md | 18 +- docs/GUI.md | 8 +- docs/SCHEMA_ALIASES.md | 2 +- src/diffractscout/benchmark.py | 1 + src/diffractscout/composition.py | 38 +- src/diffractscout/diffraction.py | 25 +- src/diffractscout/export_views.py | 127 ++++-- src/diffractscout/exporters.py | 71 +++- src/diffractscout/gui.py | 385 +++++++++++++----- src/diffractscout/models.py | 2 +- src/diffractscout/pipeline.py | 33 +- .../providers/materials_project.py | 14 +- src/diffractscout/quick_export.py | 44 +- src/diffractscout/selection.py | 7 + src/diffractscout/structure.py | 18 +- tests/test_composition.py | 11 + tests/test_export_views.py | 141 ++++++- tests/test_gui.py | 99 ++++- tests/test_parity_features.py | 19 + tests/test_pipeline.py | 81 +++- tests/test_provider_helpers.py | 4 + tests/test_quick_export.py | 40 ++ tests/test_selection.py | 10 + tests/test_structure.py | 6 +- 25 files changed, 1005 insertions(+), 210 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74f6207..703152f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,17 @@ All notable changes are recorded here. The project follows semantic versioning a - Console entry point `diffractscout-quick-export` and GUI entry point `diffractscout-gui` documented alongside `diffractscout`. - Documentation for CIF2Peaks parity features (lab Excel views, *d*-range filters, bilingual lab sheets, quick-export, figures) in README / README.zh-CN, GUI controls in `docs/GUI.md`, and module mapping in `docs/SOURCE_LINEAGE.md`. +### Fixed + +- Validated run-wide diffraction and discovery settings before copying local inputs, contacting providers, or starting GUI workers. +- Snapshotted GUI run options on the Tk thread so background workers do not access mutable Tk state. +- Required explicit overwrite authorization for existing quick-export workbooks and replaced authorized workbooks atomically. +- Rechecked bundle and benchmark targets immediately before commit so files created during a long-running calculation are not silently replaced. +- Rejected malformed or non-finite provider elasticity matrices instead of truncating oversized arrays to 6×6. +- Normalized common Unicode dash characters and case-insensitive chemical-system input during composition parsing. +- Kept the optional spglib cross-check warning-only across its 2.7-to-2.8 exception transition without emitting repeated deprecation noise. +- Exported an empty Cu Kα convenience angle, rather than a false `0°`, when a reflection is inaccessible at that wavelength. + ### Notes - Awaiting the first public GitHub release, archived software DOI, and external validation cases. diff --git a/docs/API.md b/docs/API.md index abb81c9..08cc9a0 100644 --- a/docs/API.md +++ b/docs/API.md @@ -29,14 +29,15 @@ for diagnostic in result.diagnostics: `analyze_cifs`: -1. resolves and deduplicates CIF inputs; -2. rejects overlap between input and output trees; -3. copies source artifacts into a staging result directory; -4. pairs elastic sidecars only when `include_elasticity=True`; -5. analyzes each readable phase while recording per-phase diagnostics; -6. writes CSV, optional XLSX, provenance, and a manifest; -7. verifies the staged bundle; -8. atomically moves it into the requested target. +1. validates run-wide radiation, scan, profile, and resource settings before writes; +2. resolves and deduplicates CIF inputs; +3. rejects overlap between input and output trees; +4. copies source artifacts into a staging result directory; +5. pairs elastic sidecars only when `include_elasticity=True`; +6. analyzes each readable phase while recording per-phase diagnostics; +7. writes CSV, optional XLSX, provenance, and a manifest; +8. verifies the staged bundle; +9. atomically moves it into the requested target. It returns a `PipelineResult`. An invalid phase can be recorded in `diagnostics` while other phases complete. CLI exit status is `0` when all analyzable items complete, `3` when a usable bundle contains error diagnostics for one or more items, and `2` when no phase is analyzable or a fatal input/configuration error occurs. @@ -139,6 +140,7 @@ Downloads above `confirm_above` require `authorize_large_download=True`. Automat - `diffractscout.composition.parse_composition_text(text)` - `diffractscout.composition.chemsys_subsystems(elements, max_order=None)` - `diffractscout.selection.validate_discovery_settings(settings)` +- `diffractscout.diffraction.validate_analysis_settings(settings)` - `diffractscout.structure.load_structure(path)` - `diffractscout.elasticity.discover_elastic_tensor(cif_path)` - `diffractscout.elasticity.validate_elastic_tensor(matrix_GPa, ...)` diff --git a/docs/GUI.md b/docs/GUI.md index cd79c17..5681f05 100644 --- a/docs/GUI.md +++ b/docs/GUI.md @@ -17,6 +17,10 @@ A normal Python installation with Tk support is required. On Linux, the operatin Optional extra `.[gui-dnd]` installs `tkinterdnd2` for future drag-and-drop enhancements; the current GUI does not require it. +## Layout and scrolling + +Dense forms (radiation, Cij, export options) live in **vertically scrollable** columns: use the mouse wheel or the right-hand scrollbar. Primary **Analyze / Run** actions stay **pinned under** the scroll area so they remain visible. The Activity log is in a **resizable vertical split** under the notebook—drag the sash to give the form more height on small screens. Default window size is about `1200×820` with a lower minimum (`900×640`). + ## Local CIF analysis ![Local CIF analysis interface](assets/gui-local.png) @@ -99,6 +103,8 @@ diffractscout quick-export path/to/cifs -o path/to/bundle_dir On Windows, drag CIF files or folders onto `quick_export_diffractscout.bat`. The script writes `_diffractscout.xlsx` next to the first input (bundle: `_diffractscout_bundle/`). +An existing workbook is never replaced implicitly. Choose a different path or enable the explicit overwrite option; an authorized replacement is written through a temporary file so a failed copy does not expose a partial workbook. + ## Activity log and completion states The Activity panel reports timestamps and separates informational, warning, and error diagnostics. A completed bundle can contain diagnostic errors for individual phases that failed while other phases succeeded. Completion messages therefore distinguish: @@ -111,7 +117,7 @@ The `Open result folder` action is enabled after a result bundle has been writte ## Threading and window closure -One pipeline task can run at a time. Run buttons are disabled while a worker thread is active, preventing duplicate downloads or simultaneous writes to the same target. Closing the window during a task requires confirmation. The scientific output transaction remains responsible for preserving an existing valid bundle when a run fails. +One pipeline task can run at a time. Run buttons are disabled while a worker thread is active, preventing duplicate downloads or simultaneous writes to the same target. All run options are validated and snapshotted on the GUI thread before the worker starts, so later interface edits cannot change an in-flight run and the worker never reads Tk state. Closing the window during a task requires confirmation. The scientific output transaction remains responsible for preserving an existing valid bundle when a run fails. ## Headless smoke test diff --git a/docs/SCHEMA_ALIASES.md b/docs/SCHEMA_ALIASES.md index 69a18fa..2799b45 100644 --- a/docs/SCHEMA_ALIASES.md +++ b/docs/SCHEMA_ALIASES.md @@ -49,7 +49,7 @@ DiffractScout therefore: | `d_A` | `d_spacing_A` | \(d\)-spacing in Å | | `theta_deg` | `theta_deg` | Bragg angle \(\theta\) | | `two_theta_current_deg` / `two_theta_deg` | `two_theta_deg` | \(2\theta\) for the active wavelength | -| `two_theta_cu_ka_deg` | `two_theta_cu_ka_deg` | Convenience \(2\theta\) at Cu Kα (\(\lambda=1.5406\) Å) | +| `two_theta_cu_ka_deg` | `two_theta_cu_ka_deg` | Convenience \(2\theta\) at Cu Kα (\(\lambda=1.5406\) Å); blank when the reflection is inaccessible at that wavelength | | `q_1_over_A` | `q_invA` | \(q = 2\pi / d\) | | `g_1_over_A` | `g_invA` | \(g = 1 / d\) | diff --git a/src/diffractscout/benchmark.py b/src/diffractscout/benchmark.py index 42f71d2..cebfc00 100644 --- a/src/diffractscout/benchmark.py +++ b/src/diffractscout/benchmark.py @@ -518,6 +518,7 @@ def run_reference_benchmarks( "Analytic benchmark bundle failed integrity verification: " + "; ".join(verification["errors"]) ) + _prepare_target(output_dir, overwrite=overwrite) _commit_directory(target, staging) except Exception: shutil.rmtree(staging, ignore_errors=True) diff --git a/src/diffractscout/composition.py b/src/diffractscout/composition.py index 570fe81..25851c9 100644 --- a/src/diffractscout/composition.py +++ b/src/diffractscout/composition.py @@ -32,6 +32,20 @@ r"(\d+(?:\.\d+)?)\s*([A-Z][a-z]?))\s*(?:wt%|at%|mass%|%)?", re.IGNORECASE, ) +_INPUT_TRANSLATION = str.maketrans( + { + "‐": "-", # hyphen + "‑": "-", # non-breaking hyphen + "‒": "-", # figure dash + "–": "-", # en dash + "—": "-", # em dash + "−": "-", # mathematical minus + "-": "-", # full-width hyphen-minus + "+": "+", + ",": ",", + ";": ";", + } +) def normalize_element(symbol: str) -> str | None: @@ -89,10 +103,11 @@ def parse_composition_text(text: str) -> ParsedComposition: elements: list[str] = [] labels: list[str] = [] notes: list[str] = [] - material_ids = [item.lower() for item in MP_ID_RE.findall(raw)] + normalized_raw = raw.translate(_INPUT_TRANSLATION) + material_ids = [item.lower() for item in MP_ID_RE.findall(normalized_raw)] material_ids = list(dict.fromkeys(material_ids)) - normalized_text = raw.lower().replace("+", "+").replace(",", ",") + normalized_text = normalized_raw.lower() compact = re.sub(r"[^a-z0-9]+", "", normalized_text) for alias, alias_elements in ALLOY_ALIASES.items(): if alias in compact: @@ -100,21 +115,30 @@ def parse_composition_text(text: str) -> ParsedComposition: labels.append(alias) notes.append(f"alias_element_set_only:{alias}") - for match in CHEMSYS_RE.finditer(raw): + # Accept a complete chemical-system expression case-insensitively while + # keeping formula/prose token parsing conservative. + if re.fullmatch(r"[A-Za-z]{1,2}(?:-[A-Za-z]{1,2})+", normalized_raw): + parts = normalized_raw.split("-") + normalized = [normalize_element(part) for part in parts] + if normalized and all(normalized): + _append_unique(elements, [item for item in normalized if item]) + labels.append("".join(item for item in normalized if item)) + + for match in CHEMSYS_RE.finditer(normalized_raw): parts = match.group(1).split("-") normalized = [normalize_element(part) for part in parts] if normalized and all(normalized): _append_unique(elements, [item for item in normalized if item]) labels.append(match.group(1).replace("-", "")) - for match in PERCENT_PAIR_RE.finditer(raw): + for match in PERCENT_PAIR_RE.finditer(normalized_raw): symbol = match.group(1) or match.group(4) or "" normalized = normalize_element(symbol) if normalized: _append_unique(elements, [normalized]) # Formula/grade tokens such as Ti6Al4V. Require at least two recognized elements. - for match in FORMULA_TOKEN_RE.finditer(raw.replace("-", "")): + for match in FORMULA_TOKEN_RE.finditer(normalized_raw.replace("-", "")): token = match.group(0) parsed = formula_elements(token) if len(parsed) >= 2: @@ -124,7 +148,7 @@ def parse_composition_text(text: str) -> ParsedComposition: # Explicit additive notation, e.g. Ti-6Al-4V + Cu. for match in re.finditer( r"(?:\+|+|/|、|和|加|with)\s*([A-Z][a-z]?)\b", - raw, + normalized_raw, flags=re.IGNORECASE, ): normalized = normalize_element(match.group(1)) @@ -133,7 +157,7 @@ def parse_composition_text(text: str) -> ParsedComposition: # Last-resort token parse for input such as "Ti Al V". if not elements and not material_ids: - for token in re.findall(r"\b[A-Z][a-z]?\b", raw): + for token in re.findall(r"\b[A-Z][a-z]?\b", normalized_raw): normalized = normalize_element(token) if normalized: _append_unique(elements, [normalized]) diff --git a/src/diffractscout/diffraction.py b/src/diffractscout/diffraction.py index ddecca1..bf70030 100644 --- a/src/diffractscout/diffraction.py +++ b/src/diffractscout/diffraction.py @@ -120,7 +120,13 @@ def _safe_inverse(value: float) -> float | None: return float(inverse) if np.isfinite(inverse) else None -def _validate_settings(settings: AnalysisSettings) -> None: +def validate_analysis_settings(settings: AnalysisSettings) -> None: + """Validate run-wide diffraction settings before any input or provider work. + + This is intentionally structure-independent so callers can fail fast before + copying local inputs or downloading provider records. + """ + values = ( settings.two_theta_min_deg, settings.two_theta_max_deg, @@ -162,6 +168,17 @@ def _validate_settings(settings: AnalysisSettings) -> None: value = getattr(settings, name) if isinstance(value, bool) or not isinstance(value, int) or value < 1: raise ValueError(f"{name} must be a positive integer.") + # Validate the mode-specific radiation value as part of the same preflight. + resolve_wavelength(settings) + if settings.include_figures: + from .plotting import FIGURE_EXPORT_PRESETS + + preset = settings.figure_preset or "publication" + if preset not in FIGURE_EXPORT_PRESETS: + raise ValueError( + f"Unknown figure export preset: {preset}. Valid presets: " + + ", ".join(sorted(FIGURE_EXPORT_PRESETS)) + ) def _profile_point_count(settings: AnalysisSettings) -> int: @@ -307,7 +324,7 @@ def simulate_powder_pattern( *, elastic_tensor: ElasticTensor | None = None, ) -> PhaseAnalysis: - _validate_settings(settings) + validate_analysis_settings(settings) wavelength, energy, wavelength_source = resolve_wavelength(settings) # Narrow 2θ by Bragg intersection with optional d bounds, then filter by d. settings = apply_d_range_to_settings(settings) @@ -423,7 +440,9 @@ def simulate_powder_pattern( elastic_status=elastic_status, elastic_note=elastic_note, i=index_i, - two_theta_cu_ka_deg=float(cu_ka_two_theta) if cu_ka_two_theta is not None else 0.0, + two_theta_cu_ka_deg=( + float(cu_ka_two_theta) if cu_ka_two_theta is not None else None + ), inverse_R_hkl=_safe_inverse(r_with_lp), inverse_R_hkl_no_lp=_safe_inverse(r_no_lp), sin_theta=sin_theta, diff --git a/src/diffractscout/export_views.py b/src/diffractscout/export_views.py index e2ac880..1559b06 100644 --- a/src/diffractscout/export_views.py +++ b/src/diffractscout/export_views.py @@ -10,12 +10,15 @@ import re from typing import Any -# Chinese display header -> canonical peak_rows / PEAK_HEADERS key. +# Ordered Chinese display header -> canonical peak_rows / PEAK_HEADERS key. +# Order is analysis-first: identity → hkl → geometry → intensity → ranks → elastic → meta. BEGINNER_PEAK_HEADERS_ZH: dict[str, str] = { + # Identity (filter by phase) "物相名称": "phase_name", "CIF文件": "cif_name", "化学式": "formula", "空间群": "space_group", + # Miller indices "h": "h", "k": "k", "i": "i", @@ -23,32 +26,66 @@ "晶面指标": "hkl", "晶面族": "family_label", "多重度": "multiplicity", + # Geometry "d间距_Å": "d_spacing_A", "θ_deg": "theta_deg", - "2θ_deg": "two_theta_deg", + "2θ_当前_deg": "two_theta_deg", "2θ_CuKa_deg": "two_theta_cu_ka_deg", "q_1/Å": "q_invA", "g_1/Å": "g_invA", - "相对强度": "normalized_intensity", + # Display intensity (max=100 within phase) + "相对强度_相内max100": "normalized_intensity", + "强度排序_相内": "rank_by_intensity", + # LP / no-LP channels "强度_含LP": "intensity_with_lp", "强度_无LP": "intensity_no_lp", - "体积归一强度_含LP_R_hkl": "volume_normalized_intensity_with_lp", - "体积归一强度_无LP": "volume_normalized_intensity_no_lp", - "相内相对R_hkl_%": "phase_relative_R_hkl_pct", - "强度排序": "rank_by_intensity", - "R_hkl排序": "rank_by_R_hkl", + "LP因子": "lp_factor", + # Volume-normalized J (= historical R_hkl alias) — NOT residual R + "体积归一强度J_含LP_R_hkl别名": "volume_normalized_intensity_with_lp", + "体积归一强度J_无LP": "volume_normalized_intensity_no_lp", + "1/J_含LP": "inverse_R_hkl", + "1/J_无LP": "inverse_R_hkl_no_lp", + "相内相对J_含LP_%": "phase_relative_R_hkl_pct", + "相内相对J_无LP_%": "phase_relative_R_hkl_no_lp_pct", + "J_含LP排序": "rank_by_R_hkl", + "J_无LP排序": "rank_by_R_hkl_no_lp", + # Quality flags + "是否多族共2θ": "is_multi_family_peak", + "共位hkl族数": "coincident_hkl_family_count", + # Elastic (optional) "杨氏模量_hkl法向_GPa": "young_modulus_hkl_normal_GPa", "弹性状态": "elastic_status", + "弹性备注": "elastic_note", + # Run metadata "波长_Å": "wavelength_A", + "能量_keV": "energy_keV", "晶胞体积_Å3": "cell_volume_A3", "式量_g_mol": "formula_weight_g_mol", "密度_g_cm3": "density_g_cm3", - "R_hkl说明": "r_hkl_model_note", + "J_R_hkl通道说明": "r_hkl_model_note", } +# Practical analysis column set required for criterion-1 style checks (canonical names). +ANALYSIS_PEAK_COLUMNS: tuple[str, ...] = ( + "phase_name", + "hkl", + "d_spacing_A", + "two_theta_deg", + "two_theta_cu_ka_deg", + "normalized_intensity", + "intensity_with_lp", + "intensity_no_lp", + "volume_normalized_intensity_with_lp", + "volume_normalized_intensity_no_lp", + "rank_by_intensity", + "wavelength_A", + "energy_keV", + "young_modulus_hkl_normal_GPa", +) + def beginner_peak_rows_zh(peaks: list[dict[str, Any]]) -> list[dict[str, Any]]: - """Map canonical peak row dicts onto Chinese beginner headers.""" + """Map canonical peak row dicts onto Chinese beginner headers (ordered).""" rows: list[dict[str, Any]] = [] for peak in peaks: @@ -61,37 +98,63 @@ def user_guide_rows() -> list[list[str]]: return [ ["项目", "说明"], - ["软件", "DiffractScout — 理论粉末 XRD 参考与物相候选 scout(非实验反演)"], - ["推荐峰表", "面向实验室阅读的中文精简峰表;完整英文字段见 Peaks 工作表与 peak_reference.csv"], - ["Peaks / peak_reference.csv", "完整索引峰表(规范英文列名,可复现分析)"], - ["Patterns / pattern_profiles.csv", "显示用峰形轮廓;不是仪器分辨率模型"], - ["d_spacing_A / d_A", "面间距 d(Å)"], - ["two_theta_deg", "当前波长下的 2θ(°)"], - ["two_theta_cu_ka_deg", "同一 d 在 Cu Kα(1.5406 Å)下的 2θ 便捷列"], - ["q_invA", "q = 2π/d = 4π sin(θ)/λ(1/Å)"], - ["g_invA", "g = 1/d(1/Å)"], - ["normalized_intensity / 相对强度", "相内将最强线标为 100 的显示归一;不可直接跨物相比对"], - ["intensity_with_lp", "多重度 × |F|² × Lorentz–polarization"], - ["intensity_no_lp", "多重度 × |F|²(不含 LP)"], + ["软件", "DiffractScout — 理论粉末 XRD 参考与候选相 scout(非实验反演)"], + ["建议阅读顺序", "① 本表 使用说明 → ② 推荐峰表(中文筛选)→ ③ Peaks 完整英文列 → ④ CSV 做脚本"], + ["推荐峰表", "分析向中文长表:冻结首行 + 自动筛选;用「物相名称」过滤多相;列顺序为 身份→hkl→几何→强度→排序→弹性"], + ["Peaks", "完整规范英文峰表(与 peak_reference.csv 列名一致);同样冻结首行 + 自动筛选"], + ["Phases", "每个 CIF/相的晶胞、空间群、占位警告、弹性配对状态"], + ["Patterns", "显示用连续峰形(伪 Voigt 等);不是仪器分辨率或背景模型"], + ["Elasticity", "6×6 Cij(GPa)与坐标框架、来源记录"], + ["Diagnostics", "结构化警告/失败;部分相失败时仍可能有可用峰表"], + [], + ["—— 几何列 ——", ""], + ["d间距_Å / d_spacing_A", "面间距 d,单位 Å;跨波长比较优先看 d"], + ["θ_deg / theta_deg", "Bragg 角 θ(°);衍射角 2θ = 2×θ"], + ["2θ_当前_deg / two_theta_deg", "当前导出波长/能量下的 2θ(°);与实验横坐标对齐时用此列"], + ["2θ_CuKa_deg / two_theta_cu_ka_deg", "同一 d 换算到 Cu Kα(λ=1.5406 Å)的 2θ,便于对照常见实验室数据"], + ["q_1/Å / q_invA", "q = 2π/d = 4π sin(θ)/λ,单位 1/Å"], + ["g_1/Å / g_invA", "g = 1/d,单位 1/Å"], + [], + ["—— 强度列(理论) ——", ""], + ["相对强度_相内max100 / normalized_intensity", "相内最强线标为 100 的显示归一;不可跨物相直接比绝对强度"], + ["强度_含LP / intensity_with_lp", "多重度 × |F_xray|² × Lorentz–polarization(LP)"], + ["强度_无LP / intensity_no_lp", "多重度 × |F_xray|²(不含 LP)"], + ["LP因子 / lp_factor", "经典粉末 LP:(1+cos²2θ)/(sin²θ cosθ)"], [ - "volume_normalized_intensity_with_lp / R_hkl", - "I_with_LP / V_cell² — 项目定义的体积归一理论强度(历史别名 material_scattering_factor_R_hkl)", + "体积归一强度J_含LP / volume_normalized_intensity_with_lp", + "J = I_with_LP / V_cell²;历史别名 material_scattering_factor_R_hkl / R_hkl", ], [ - "volume_normalized_intensity_no_lp", - "I_no_LP / V_cell² — 同上通道但不含 LP(历史别名 material_scattering_factor_R_hkl_no_lp)", + "体积归一强度J_无LP / volume_normalized_intensity_no_lp", + "J = I_no_LP / V_cell²;历史别名 material_scattering_factor_R_hkl_no_lp", ], + ["1/J_含LP / inverse_R_hkl", "1 / J_with_LP(便利列;非 QPA 系数)"], + ["1/J_无LP / inverse_R_hkl_no_lp", "1 / J_no_LP"], + ["相内相对J_%", "同一相内按最大 J 归一到 100%"], + ["强度排序 / rank_by_*", "相内 1-based 排名:1 为最强,数值越小越强(勿按 rank 降序找强峰)"], + [], + ["—— 重要科学边界 ——", ""], [ - "重要:R_hkl 不是残差", - "R_hkl 不是 Rietveld R / Rwp / RBragg 等晶体学残差因子,也不是标准化 QPA 系数或实验标定散射因子", + "R_hkl / J 不是残差", + "R_hkl 风格名称只是体积归一理论强度别名,不是 Rietveld R/Rwp/RBragg 等晶体学残差,也不是标准化 QPA 或实验标定散射因子", ], [ "本软件不做", - "物相鉴定、Rietveld/Le Bail/Pawley 精修、定量相分析(QPA)、绝对强度标定、择优取向/吸收/背底推断", + "实验物相自动鉴定、Rietveld/Le Bail/Pawley、定量相分析(QPA)、绝对强度标定、择优取向/吸收/背底/微结构反演", ], - ["杨氏模量列", "可选:沿 hkl 倒易法向的 E(n);依赖匹配的弹性张量与坐标框架"], - ["scientific_boundary", "完整科学边界见 provenance.json 与 SCIENTIFIC_CONTRACTS.md"], - ["复现", "使用同一 CIF SHA-256、波长设置与软件版本;manifest.json 提供文件 SHA-256 清单"], + ["是否多族共2θ", "TRUE 表示相近 2θ 上有多个 hkl 族;对比实验时需谨慎归属"], + [], + ["—— 弹性(可选) ——", ""], + ["杨氏模量_hkl法向_GPa", "沿 hkl 倒易法向的 E(n)=1/(n^T S n);需有效 Cij 与兼容坐标框架"], + ["弹性状态", "valid / not_available / frame_transform_required / invalid 等;非 valid 时模量列可为空"], + [], + ["—— 如何在 Excel 中分析 ——", ""], + ["筛选物相", "在 推荐峰表 或 Peaks 对「物相名称/phase_name」列使用自动筛选"], + ["找强峰", "按 相对强度 降序;或筛选 rank_by_intensity=1..5(rank 升序,1 最强)"], + ["对齐实验 2θ", "波长一致时用 2θ_当前;波长不同先比 d 或看 2θ_CuKa"], + ["跨相强度", "不要用相内相对强度直接定量;本表为理论参考"], + ["复现", "同一 CIF SHA-256、波长设置与软件版本;manifest.json 提供文件哈希"], + ["scientific_boundary", "完整边界见 provenance.json 与 docs/SCIENTIFIC_CONTRACTS.md"], ] diff --git a/src/diffractscout/exporters.py b/src/diffractscout/exporters.py index 4fb8bfe..b29d3c1 100644 --- a/src/diffractscout/exporters.py +++ b/src/diffractscout/exporters.py @@ -15,6 +15,7 @@ from .diffraction import SCIENTIFIC_BOUNDARY from .export_views import ( + ANALYSIS_PEAK_COLUMNS, BEGINNER_PEAK_HEADERS_ZH, beginner_peak_rows_zh, safe_excel_sheet_title, @@ -78,12 +79,16 @@ "warnings", "source_metadata", ] +# Analysis-first order: identity → hkl → geometry → intensity → ranks → SF extras → elastic → meta. +# All historical column names retained (additive reordering only). PEAK_HEADERS = [ + # Identity (multi-phase filter) "phase_name", "cif_name", "cif_sha256", "formula", "space_group", + # Miller "h", "k", "i", @@ -91,23 +96,20 @@ "hkl", "family_label", "multiplicity", + # Geometry "d_spacing_A", "theta_deg", "two_theta_deg", "two_theta_cu_ka_deg", "q_invA", "g_invA", - "sin_theta", - "cos_theta", - "sin_theta_over_lambda", - "sin2_theta_over_lambda2", - "structure_factor_sq", - "mean_structure_factor_sq_per_multiplicity", - "mean_structure_factor_abs_per_multiplicity", + # Display + LP channels + "normalized_intensity", + "rank_by_intensity", + "intensity_with_lp", "intensity_no_lp", "lp_factor", - "intensity_with_lp", - "normalized_intensity", + # Volume-normalized J (+ legacy R_hkl aliases) "volume_normalized_intensity_with_lp", "volume_normalized_intensity_no_lp", "material_scattering_factor_R_hkl", @@ -116,11 +118,19 @@ "inverse_R_hkl_no_lp", "phase_relative_R_hkl_pct", "phase_relative_R_hkl_no_lp_pct", - "rank_by_intensity", "rank_by_R_hkl", "rank_by_R_hkl_no_lp", + # Structure-factor detail + "structure_factor_sq", + "mean_structure_factor_sq_per_multiplicity", + "mean_structure_factor_abs_per_multiplicity", + "sin_theta", + "cos_theta", + "sin_theta_over_lambda", + "sin2_theta_over_lambda2", "is_multi_family_peak", "coincident_hkl_family_count", + # Elastic + run meta "young_modulus_hkl_normal_GPa", "elastic_status", "elastic_note", @@ -512,8 +522,11 @@ def _add_sheet( sheet.append([_cell_value(row.get(header)) for header in headers]) else: sheet.append(["no rows", *([""] * (len(headers) - 1))]) + # Frozen header + autofilter: required for analysis-ready long tables. sheet.freeze_panes = "A2" - sheet.auto_filter.ref = sheet.dimensions + last_col = get_column_letter(max(len(headers), 1)) + last_row = max(sheet.max_row, 1) + sheet.auto_filter.ref = f"A1:{last_col}{last_row}" header_fill = PatternFill("solid", fgColor="16324F") for cell in sheet[1]: cell.font = Font(bold=True, color="FFFFFF") @@ -521,7 +534,8 @@ def _add_sheet( cell.alignment = Alignment(horizontal="center", vertical="center", wrap_text=True) for column_index, header in enumerate(headers, start=1): sampled = [str(header)] + [str(_cell_value(row.get(header))) for row in rows[:200]] - width = min(max(max(len(value) for value in sampled) + 2, 10), 42) + # Slightly wider for analysis headers with units in the name. + width = min(max(max(len(value) for value in sampled) + 2, 10), 48) sheet.column_dimensions[get_column_letter(column_index)].width = width for row in sheet.iter_rows(min_row=2): for cell in row: @@ -570,9 +584,14 @@ def write_excel_workbook( ) -> Path: workbook = Workbook() workbook.remove(workbook.active) + # Lab-first when enabled: guide → Chinese long table → full Peaks, then metadata sheets. + if export_lab_views: + _add_guide_sheet(workbook, "使用说明", user_guide_rows()) + zh_headers = list(BEGINNER_PEAK_HEADERS_ZH.keys()) + _add_sheet(workbook, "推荐峰表", beginner_peak_rows_zh(peaks), zh_headers) _add_sheet(workbook, "Summary", summary, SUMMARY_HEADERS) - _add_sheet(workbook, "Phases", phases, PHASE_HEADERS) _add_sheet(workbook, "Peaks", peaks, PEAK_HEADERS) + _add_sheet(workbook, "Phases", phases, PHASE_HEADERS) _add_sheet(workbook, "Elasticity", elasticity, ELASTICITY_HEADERS) _add_sheet(workbook, "Candidates", candidates, CANDIDATE_HEADERS) _add_sheet(workbook, "Downloads", downloads, DOWNLOAD_HEADERS) @@ -580,9 +599,6 @@ def write_excel_workbook( if include_patterns: _add_sheet(workbook, "Patterns", patterns, PATTERN_HEADERS) if export_lab_views: - zh_headers = list(BEGINNER_PEAK_HEADERS_ZH.keys()) - _add_sheet(workbook, "推荐峰表", beginner_peak_rows_zh(peaks), zh_headers) - _add_guide_sheet(workbook, "使用说明", user_guide_rows()) analysis_list = analyses or [] if 0 < len(analysis_list) <= 20: used_titles: set[str] = set(workbook.sheetnames) @@ -594,6 +610,9 @@ def write_excel_workbook( continue title = safe_excel_sheet_title(f"峰_{analysis.phase_name}", used=used_titles) _add_sheet(workbook, title, phase_peaks, PEAK_HEADERS) + # Open on the Chinese analysis long table when present. + if "推荐峰表" in workbook.sheetnames: + workbook.active = workbook["推荐峰表"] path.parent.mkdir(parents=True, exist_ok=True) temporary = path.with_suffix(path.suffix + ".tmp") try: @@ -625,6 +644,17 @@ def _summary_rows( {"key": "d_max_A", "value": settings.d_max_A}, {"key": "pattern_axis", "value": settings.pattern_axis}, {"key": "export_lab_views", "value": settings.export_lab_views}, + { + "key": "peak_longtable_note", + "value": ( + "Peaks sheet: analysis-first column order; freeze header + autofilter; " + "filter phase_name for multi-phase. Lab views add 推荐峰表 + 使用说明." + ), + }, + { + "key": "analysis_peak_columns", + "value": list(ANALYSIS_PEAK_COLUMNS), + }, {"key": "scientific_boundary", "value": SCIENTIFIC_BOUNDARY}, ] @@ -759,11 +789,12 @@ def export_result_bundle( ), "elastic_modulus": "E(n) = 1 / (q(n)^T S q(n)) under engineering-shear Voigt convention", "lab_views_schema": ( - "When export_lab_views is true, results.xlsx adds 推荐峰表 (Chinese beginner headers " - "mapped by BEGINNER_PEAK_HEADERS_ZH), 使用说明 (two-column guide), and optional " - "per-phase peak sheets (≤20 phases). Canonical English sheets and CSV columns remain " - "the machine-readable schema; lab views are additive presentation only." + "When export_lab_views is true, results.xlsx opens on 推荐峰表 after 使用说明: " + "analysis-first Chinese long table (BEGINNER_PEAK_HEADERS_ZH) with freeze+autofilter, " + "optional per-phase peak sheets (≤20 phases). Peaks/CSV remain the English machine schema " + "with the same peak_rows values; R_hkl names are volume-normalized J aliases, not residuals." ), + "analysis_peak_columns": list(ANALYSIS_PEAK_COLUMNS), "pattern_axis_columns": ( "pattern_profiles.csv always includes two_theta_deg, d_A, q_invA, g_invA, " "x_axis_mode (settings.pattern_axis), x (selected axis value), and relative_intensity" diff --git a/src/diffractscout/gui.py b/src/diffractscout/gui.py index 137b948..c58e7f3 100644 --- a/src/diffractscout/gui.py +++ b/src/diffractscout/gui.py @@ -14,11 +14,13 @@ from typing import Any, Callable, Mapping from . import __version__ +from .diffraction import validate_analysis_settings from .elasticity_input import parse_cij_matrix_6x6, parse_cij_paste_text, parse_cubic_cij from .gui_i18n import DEFAULT_LANG, t from .models import AnalysisSettings, DiscoverySettings, ElasticTensor, PipelineResult from .pipeline import analyze_cifs, run_pipeline from .providers.materials_project import MaterialsProjectProvider +from .selection import validate_discovery_settings try: # Tk remains optional on minimal/headless Python installations. import tkinter as tk @@ -114,7 +116,7 @@ def analysis_settings_from_form(values: Mapping[str, object]) -> AnalysisSetting pattern_axis = str(values.get("pattern_axis", "two_theta")).strip() if pattern_axis not in _PATTERN_AXES: raise ValueError(f"Pattern axis must be one of: {', '.join(_PATTERN_AXES)}.") - return AnalysisSettings( + settings = AnalysisSettings( input_mode=mode, # type: ignore[arg-type] source_preset=str(values.get("source_preset", "Cu Ka")), wavelength_A=radiation @@ -141,13 +143,15 @@ def analysis_settings_from_form(values: Mapping[str, object]) -> AnalysisSetting export_lab_views=_as_bool(values.get("export_lab_views", True), True), include_patterns=_as_bool(values.get("include_patterns", True), True), ) + validate_analysis_settings(settings) + return settings def discovery_settings_from_form(values: Mapping[str, object]) -> DiscoverySettings: mode = str(values.get("mode", "possible_phases")).strip() if mode not in {"possible_phases", "near_stable", "single_chemsys", "mpids_only"}: raise ValueError("Unknown discovery mode.") - return DiscoverySettings( + settings = DiscoverySettings( mode=mode, # type: ignore[arg-type] e_hull_max_eV_atom=_optional_float(values.get("e_hull_max"), "Maximum energy above hull"), max_subsystem_order=_optional_int(values.get("max_subsystem_order"), "Maximum subsystem order"), @@ -155,6 +159,8 @@ def discovery_settings_from_form(values: Mapping[str, object]) -> DiscoverySetti max_total=_optional_int(values.get("max_total"), "Maximum candidates"), exclude_deprecated=not bool(values.get("include_deprecated", False)), ) + validate_discovery_settings(settings) + return settings def open_path(path: str | Path) -> None: @@ -190,8 +196,9 @@ class DiffractScoutApp(_TkBase): # type: ignore[valid-type,misc] def __init__(self) -> None: super().__init__() self.title(f"DiffractScout {__version__}") - self.geometry("1180x920") - self.minsize(980, 780) + # Prefer a size that fits common 1080p / laptop viewports; content scrolls. + self.geometry("1200x820") + self.minsize(900, 640) self.configure(background=BG) self.protocol("WM_DELETE_WINDOW", self._on_close) @@ -208,17 +215,19 @@ def __init__(self) -> None: self._title_pairs: list[tuple[Any, Any, str, str]] = [] self._labelframes: list[tuple[Any, str]] = [] self._notebook_tabs: list[tuple[int, str]] = [] + self._wrap_labels: list[tuple[Any, int]] = [] + self._scroll_canvases: list[Any] = [] self._syncing_shortcut = False self._configure_style() self._create_variables() self._build_header() self._build_status_bar() - self._build_activity_panel() - self._build_body() + self._build_main_split() self._sync_radiation_controls() self._refresh_cij_status() self._apply_language() + self.bind("", self._on_root_configure, add="+") self.after(120, self._poll) self._log(self._t("log_ready"), "info") @@ -241,11 +250,11 @@ def _configure_style(self) -> None: style.configure("Title.TLabel", background=CARD, foreground=NAVY, font=("Segoe UI Semibold", 12)) style.configure("Hint.TLabel", background=CARD, foreground=MUTED, font=("Segoe UI", 8)) style.configure("Header.TLabel", background=NAVY_DARK, foreground="white") - style.configure("HeaderTitle.TLabel", background=NAVY_DARK, foreground="white", font=("Segoe UI Semibold", 22)) - style.configure("HeaderSub.TLabel", background=NAVY_DARK, foreground="#BFD3E5", font=("Segoe UI", 10)) - style.configure("Badge.TLabel", background=TEAL, foreground="white", font=("Segoe UI Semibold", 9), padding=(9, 4)) + style.configure("HeaderTitle.TLabel", background=NAVY_DARK, foreground="white", font=("Segoe UI Semibold", 18)) + style.configure("HeaderSub.TLabel", background=NAVY_DARK, foreground="#BFD3E5", font=("Segoe UI", 9)) + style.configure("Badge.TLabel", background=TEAL, foreground="white", font=("Segoe UI Semibold", 9), padding=(8, 3)) style.configure("TNotebook", background=BG, borderwidth=0) - style.configure("TNotebook.Tab", background="#DCE6EE", foreground=NAVY, padding=(18, 9), font=("Segoe UI Semibold", 9)) + style.configure("TNotebook.Tab", background="#DCE6EE", foreground=NAVY, padding=(14, 6), font=("Segoe UI Semibold", 9)) style.map("TNotebook.Tab", background=[("selected", CARD)], foreground=[("selected", TEAL_DARK)]) style.configure("Primary.TButton", background=TEAL_DARK, foreground="white", padding=(14, 9), font=("Segoe UI Semibold", 10), borderwidth=0) style.map("Primary.TButton", background=[("active", TEAL), ("disabled", "#9FB3C3")]) @@ -310,26 +319,26 @@ def _create_variables(self) -> None: self.input_count_text = tk.StringVar(value=self._t("inputs_none")) def _build_header(self) -> None: - header = tk.Frame(self, bg=NAVY_DARK, height=102) + header = tk.Frame(self, bg=NAVY_DARK, height=78) header.pack(fill="x") header.pack_propagate(False) - logo = tk.Canvas(header, width=70, height=70, bg=NAVY_DARK, highlightthickness=0) - logo.pack(side="left", padx=(24, 10), pady=15) - logo.create_oval(8, 8, 62, 62, outline=TEAL, width=3) - for x, y in ((21, 24), (48, 22), (28, 48), (50, 47)): - logo.create_oval(x - 4, y - 4, x + 4, y + 4, fill="white", outline="") - logo.create_line(21, 24, 48, 22, 50, 47, 28, 48, 21, 24, fill="#8BC6D5", width=2) + logo = tk.Canvas(header, width=52, height=52, bg=NAVY_DARK, highlightthickness=0) + logo.pack(side="left", padx=(18, 8), pady=12) + logo.create_oval(4, 4, 48, 48, outline=TEAL, width=2) + for x, y in ((16, 18), (36, 16), (20, 36), (38, 35)): + logo.create_oval(x - 3, y - 3, x + 3, y + 3, fill="white", outline="") + logo.create_line(16, 18, 36, 16, 38, 35, 20, 36, 16, 18, fill="#8BC6D5", width=2) text = tk.Frame(header, bg=NAVY_DARK) - text.pack(side="left", fill="y", pady=15) + text.pack(side="left", fill="y", pady=10) ttk.Label(text, text="DiffractScout", style="HeaderTitle.TLabel").pack(anchor="w") self.header_sub = ttk.Label(text, text=self._t("app_subtitle"), style="HeaderSub.TLabel") - self.header_sub.pack(anchor="w", pady=(3, 0)) + self.header_sub.pack(anchor="w", pady=(2, 0)) self._i18n_targets.append((self.header_sub, "app_subtitle", "text")) right = tk.Frame(header, bg=NAVY_DARK) - right.pack(side="right", padx=24) - ttk.Label(right, text=f"v{__version__}", style="Badge.TLabel").pack(anchor="e", pady=(8, 6)) + right.pack(side="right", padx=18) + ttk.Label(right, text=f"v{__version__}", style="Badge.TLabel").pack(anchor="e", pady=(6, 4)) lang_row = tk.Frame(right, bg=NAVY_DARK) lang_row.pack(anchor="e") self.lang_label = ttk.Label(lang_row, text=self._t("lang_label"), style="HeaderSub.TLabel") @@ -345,28 +354,108 @@ def _build_header(self) -> None: lang_box.pack(side="left") lang_box.bind("<>", lambda _e: self._set_language(self.lang_var.get())) - def _build_body(self) -> None: - body = ttk.Frame(self, padding=(18, 14, 18, 4)) - body.pack(fill="both", expand=True) - notebook = ttk.Notebook(body) + def _build_main_split(self) -> None: + """Notebook above, resizable activity log below; both share remaining height.""" + + paned = ttk.Panedwindow(self, orient="vertical") + paned.pack(fill="both", expand=True) + self._main_paned = paned + + body_host = ttk.Frame(paned, padding=(14, 10, 14, 4)) + activity_host = ttk.Frame(paned, padding=(14, 2, 14, 0)) + paned.add(body_host, weight=5) + paned.add(activity_host, weight=1) + + notebook = ttk.Notebook(body_host) self.notebook = notebook notebook.pack(fill="both", expand=True) - local = ttk.Frame(notebook, style="Card.TFrame", padding=16) - mp = ttk.Frame(notebook, style="Card.TFrame", padding=16) + local = ttk.Frame(notebook, style="Card.TFrame", padding=10) + mp = ttk.Frame(notebook, style="Card.TFrame", padding=10) notebook.add(local, text=self._t("tab_local")) notebook.add(mp, text=self._t("tab_mp")) self._notebook_tabs = [(0, "tab_local"), (1, "tab_mp")] self._build_local_tab(local) self._build_mp_tab(mp) + self._build_activity_panel(activity_host) + # Give the form most of the space after first layout pass. + self.after(80, self._set_default_sash) + + def _set_default_sash(self) -> None: + try: + height = max(self.winfo_height(), 640) + # Leave ~150–200 px for the activity log band. + sash = max(360, height - 220) + self._main_paned.sashpos(0, sash) + except Exception: # pragma: no cover - geometry timing + return + + def _make_scrollable(self, parent: Any, *, bg: str = CARD) -> tuple[Any, Any]: + """Return (outer_frame, interior_frame) with vertical scrollbar + mouse wheel.""" + + outer = ttk.Frame(parent, style="Card.TFrame") + canvas = tk.Canvas(outer, bg=bg, highlightthickness=0, bd=0) + scrollbar = ttk.Scrollbar(outer, orient="vertical", command=canvas.yview) + canvas.configure(yscrollcommand=scrollbar.set) + scrollbar.pack(side="right", fill="y") + canvas.pack(side="left", fill="both", expand=True) + + interior = ttk.Frame(canvas, style="Card.TFrame") + window_id = canvas.create_window((0, 0), window=interior, anchor="nw") + self._scroll_canvases.append(canvas) + + def _sync_scrollregion(_event: object | None = None) -> None: + canvas.configure(scrollregion=canvas.bbox("all")) + + def _sync_width(event: Any) -> None: + canvas.itemconfigure(window_id, width=max(int(event.width), 1)) + + interior.bind("", _sync_scrollregion) + canvas.bind("", _sync_width) + + def _on_wheel(event: Any) -> str | None: + if not canvas.winfo_exists(): + return None + # Only scroll if content overflows. + if canvas.bbox("all") is None: + return None + top, bottom = canvas.yview() + if top <= 0.0 and bottom >= 1.0: + return None + delta = int(getattr(event, "delta", 0) or 0) + if delta: + canvas.yview_scroll(int(-delta / 120), "units") + elif getattr(event, "num", None) == 4: + canvas.yview_scroll(-3, "units") + elif getattr(event, "num", None) == 5: + canvas.yview_scroll(3, "units") + return "break" + + def _bind_recursive(widget: Any) -> None: + widget.bind("", _on_wheel, add="+") + widget.bind("", _on_wheel, add="+") + widget.bind("", _on_wheel, add="+") + for child in widget.winfo_children(): + _bind_recursive(child) + + def _bind_tree(_event: object | None = None) -> None: + _bind_recursive(interior) + canvas.bind("", _on_wheel, add="+") + canvas.bind("", _on_wheel, add="+") + canvas.bind("", _on_wheel, add="+") + + interior.bind("", lambda _e: self.after_idle(_bind_tree), add="+") + self.after_idle(_bind_tree) + return outer, interior def _card_title(self, parent: Any, title_key: str, hint_key: str) -> None: title = ttk.Label(parent, text=self._t(title_key), style="Title.TLabel") title.pack(anchor="w") - hint = ttk.Label(parent, text=self._t(hint_key), style="Hint.TLabel", wraplength=480) - hint.pack(anchor="w", pady=(2, 10)) + hint = ttk.Label(parent, text=self._t(hint_key), style="Hint.TLabel", wraplength=420) + hint.pack(anchor="w", pady=(2, 8)) self._title_pairs.append((title, hint, title_key, hint_key)) self._i18n_targets.append((title, title_key, "text")) self._i18n_targets.append((hint, hint_key, "text")) + self._wrap_labels.append((hint, 420)) def _labeled_frame(self, parent: Any, key: str, **kwargs: Any) -> ttk.LabelFrame: frame = ttk.LabelFrame(parent, text=self._t(key), **kwargs) @@ -374,16 +463,19 @@ def _labeled_frame(self, parent: Any, key: str, **kwargs: Any) -> ttk.LabelFrame return frame def _build_local_tab(self, frame: ttk.Frame) -> None: - frame.columnconfigure(0, weight=1) - frame.columnconfigure(1, weight=1) + frame.columnconfigure(0, weight=1, minsize=280) + frame.columnconfigure(1, weight=2, minsize=360) frame.rowconfigure(0, weight=1) - left = ttk.Frame(frame, style="Card.TFrame", padding=(0, 0, 12, 0)) - right = ttk.Frame(frame, style="Card.TFrame", padding=(12, 0, 0, 0)) + left = ttk.Frame(frame, style="Card.TFrame", padding=(0, 0, 10, 0)) + right_shell = ttk.Frame(frame, style="Card.TFrame", padding=(10, 0, 0, 0)) left.grid(row=0, column=0, sticky="nsew") - right.grid(row=0, column=1, sticky="nsew") - self._card_title(left, "local_select_title", "local_select_hint") + right_shell.grid(row=0, column=1, sticky="nsew") + right_shell.rowconfigure(0, weight=1) + right_shell.columnconfigure(0, weight=1) + # Left: list grows; buttons stay visible. + self._card_title(left, "local_select_title", "local_select_hint") list_frame = tk.Frame(left, bg=CARD, highlightbackground=BORDER, highlightthickness=1) list_frame.pack(fill="both", expand=True) self.input_list = tk.Listbox( @@ -405,47 +497,55 @@ def _build_local_tab(self, frame: ttk.Frame) -> None: ttk.Label(left, textvariable=self.input_count_text, style="Hint.TLabel").pack(anchor="w", pady=(5, 4)) buttons = ttk.Frame(left, style="Card.TFrame") - buttons.pack(fill="x", pady=(0, 12)) + buttons.pack(fill="x", pady=(0, 8)) + buttons.columnconfigure(0, weight=1) + buttons.columnconfigure(1, weight=1) self.btn_add_cif = ttk.Button( buttons, text=self._t("btn_add_cif"), style="Secondary.TButton", command=self._add_cif_files ) - self.btn_add_cif.pack(side="left", padx=(0, 6)) + self.btn_add_cif.grid(row=0, column=0, sticky="ew", padx=(0, 4), pady=2) self._register_text(self.btn_add_cif, "btn_add_cif") self.btn_add_folder = ttk.Button( buttons, text=self._t("btn_add_folder"), style="Secondary.TButton", command=self._add_cif_folder ) - self.btn_add_folder.pack(side="left", padx=6) + self.btn_add_folder.grid(row=0, column=1, sticky="ew", padx=(4, 0), pady=2) self._register_text(self.btn_add_folder, "btn_add_folder") self.btn_remove = ttk.Button( buttons, text=self._t("btn_remove"), style="Danger.TButton", command=self._remove_inputs ) - self.btn_remove.pack(side="left", padx=6) + self.btn_remove.grid(row=1, column=0, sticky="ew", padx=(0, 4), pady=2) self._register_text(self.btn_remove, "btn_remove") self.btn_clear = ttk.Button( buttons, text=self._t("btn_clear"), style="Secondary.TButton", command=self._clear_inputs ) - self.btn_clear.pack(side="left", padx=6) + self.btn_clear.grid(row=1, column=1, sticky="ew", padx=(4, 0), pady=2) self._register_text(self.btn_clear, "btn_clear") - output_box = self._labeled_frame(left, "result_bundle", padding=10) + output_box = self._labeled_frame(left, "result_bundle", padding=8) output_box.pack(fill="x") self._path_entry(output_box, self.local_output, self._choose_local_output) self.chk_recursive = ttk.Checkbutton( output_box, text=self._t("scan_recursive"), variable=self.local_recursive ) - self.chk_recursive.pack(anchor="w", pady=(8, 0)) + self.chk_recursive.pack(anchor="w", pady=(6, 0)) self._register_text(self.chk_recursive, "scan_recursive") self.chk_overwrite_local = ttk.Checkbutton( output_box, text=self._t("overwrite_bundle"), variable=self.overwrite ) - self.chk_overwrite_local.pack(anchor="w", pady=(4, 0)) + self.chk_overwrite_local.pack(anchor="w", pady=(2, 0)) self._register_text(self.chk_overwrite_local, "overwrite_bundle") + # Right: scrollable form + pinned primary action. + scroll_outer, right = self._make_scrollable(right_shell) + scroll_outer.grid(row=0, column=0, sticky="nsew") + footer = ttk.Frame(right_shell, style="Card.TFrame") + footer.grid(row=1, column=0, sticky="ew", pady=(8, 0)) + self._card_title(right, "scientific_title", "scientific_hint") self._analysis_controls(right) self._build_cij_panel(right) - options = self._labeled_frame(right, "outputs", padding=10) - options.pack(fill="x", pady=(10, 0)) + options = self._labeled_frame(right, "outputs", padding=8) + options.pack(fill="x", pady=(8, 0)) self.chk_elasticity_local = ttk.Checkbutton( options, text=self._t("pair_elasticity"), variable=self.include_elasticity ) @@ -471,70 +571,83 @@ def _build_local_tab(self, frame: ttk.Frame) -> None: ) self.chk_figures.pack(anchor="w") self._register_text(self.chk_figures, "include_figures") + button = ttk.Button( - right, text=self._t("analyze_local"), style="Primary.TButton", command=self._run_local + footer, text=self._t("analyze_local"), style="Primary.TButton", command=self._run_local ) - button.pack(fill="x", pady=(14, 0)) + button.pack(fill="x") self._register_text(button, "analyze_local") self._run_buttons.append(button) def _build_mp_tab(self, frame: ttk.Frame) -> None: - frame.columnconfigure(0, weight=1) - frame.columnconfigure(1, weight=1) + frame.columnconfigure(0, weight=1, minsize=280) + frame.columnconfigure(1, weight=2, minsize=360) frame.rowconfigure(0, weight=1) - left = ttk.Frame(frame, style="Card.TFrame", padding=(0, 0, 12, 0)) - right = ttk.Frame(frame, style="Card.TFrame", padding=(12, 0, 0, 0)) - left.grid(row=0, column=0, sticky="nsew") - right.grid(row=0, column=1, sticky="nsew") + + left_shell = ttk.Frame(frame, style="Card.TFrame", padding=(0, 0, 10, 0)) + right_shell = ttk.Frame(frame, style="Card.TFrame", padding=(10, 0, 0, 0)) + left_shell.grid(row=0, column=0, sticky="nsew") + right_shell.grid(row=0, column=1, sticky="nsew") + left_shell.rowconfigure(0, weight=1) + left_shell.columnconfigure(0, weight=1) + right_shell.rowconfigure(0, weight=1) + right_shell.columnconfigure(0, weight=1) + + left_scroll, left = self._make_scrollable(left_shell) + left_scroll.grid(row=0, column=0, sticky="nsew") + right_scroll, right = self._make_scrollable(right_shell) + right_scroll.grid(row=0, column=0, sticky="nsew") + footer = ttk.Frame(right_shell, style="Card.TFrame") + footer.grid(row=1, column=0, sticky="ew", pady=(8, 0)) self._card_title(left, "mp_discover_title", "mp_discover_hint") - form = self._labeled_frame(left, "mp_query", padding=12) + form = self._labeled_frame(left, "mp_query", padding=10) form.pack(fill="x") self.lbl_composition = ttk.Label(form, text=self._t("composition"), style="Card.TLabel") - self.lbl_composition.grid(row=0, column=0, sticky="w", pady=5) + self.lbl_composition.grid(row=0, column=0, sticky="w", pady=4) self._register_text(self.lbl_composition, "composition") ttk.Entry(form, textvariable=self.mp_composition).grid( - row=0, column=1, columnspan=3, sticky="ew", padx=(8, 0), pady=5 + row=0, column=1, columnspan=3, sticky="ew", padx=(8, 0), pady=4 ) self.lbl_api_key = ttk.Label(form, text=self._t("api_key"), style="Card.TLabel") - self.lbl_api_key.grid(row=1, column=0, sticky="w", pady=5) + self.lbl_api_key.grid(row=1, column=0, sticky="w", pady=4) self._register_text(self.lbl_api_key, "api_key") self.mp_key_entry = ttk.Entry(form, textvariable=self.mp_key, show="" if self.mp_show_key.get() else "•") - self.mp_key_entry.grid(row=1, column=1, columnspan=2, sticky="ew", padx=(8, 8), pady=5) + self.mp_key_entry.grid(row=1, column=1, columnspan=2, sticky="ew", padx=(8, 8), pady=4) self.chk_show_key = ttk.Checkbutton( form, text=self._t("show_key"), variable=self.mp_show_key, command=self._toggle_key ) self.chk_show_key.grid(row=1, column=3, sticky="w") self._register_text(self.chk_show_key, "show_key") self.lbl_mode = ttk.Label(form, text=self._t("mode"), style="Card.TLabel") - self.lbl_mode.grid(row=2, column=0, sticky="w", pady=5) + self.lbl_mode.grid(row=2, column=0, sticky="w", pady=4) self._register_text(self.lbl_mode, "mode") ttk.Combobox( form, textvariable=self.mp_mode, values=("possible_phases", "near_stable", "single_chemsys", "mpids_only"), state="readonly", - ).grid(row=2, column=1, sticky="ew", padx=(8, 12), pady=5) + ).grid(row=2, column=1, sticky="ew", padx=(8, 12), pady=4) self.lbl_ehull = ttk.Label(form, text=self._t("e_hull_max"), style="Card.TLabel") - self.lbl_ehull.grid(row=2, column=2, sticky="w", pady=5) + self.lbl_ehull.grid(row=2, column=2, sticky="w", pady=4) self._register_text(self.lbl_ehull, "e_hull_max") - ttk.Entry(form, textvariable=self.mp_e_hull).grid(row=2, column=3, sticky="ew", padx=(8, 0), pady=5) + ttk.Entry(form, textvariable=self.mp_e_hull).grid(row=2, column=3, sticky="ew", padx=(8, 0), pady=4) self.lbl_sub_order = ttk.Label(form, text=self._t("subsystem_order"), style="Card.TLabel") - self.lbl_sub_order.grid(row=3, column=0, sticky="w", pady=5) + self.lbl_sub_order.grid(row=3, column=0, sticky="w", pady=4) self._register_text(self.lbl_sub_order, "subsystem_order") ttk.Entry(form, textvariable=self.mp_subsystem_order).grid( - row=3, column=1, sticky="ew", padx=(8, 12), pady=5 + row=3, column=1, sticky="ew", padx=(8, 12), pady=4 ) self.lbl_per_sub = ttk.Label(form, text=self._t("per_subsystem"), style="Card.TLabel") - self.lbl_per_sub.grid(row=3, column=2, sticky="w", pady=5) + self.lbl_per_sub.grid(row=3, column=2, sticky="w", pady=4) self._register_text(self.lbl_per_sub, "per_subsystem") ttk.Entry(form, textvariable=self.mp_per_subsystem).grid( - row=3, column=3, sticky="ew", padx=(8, 0), pady=5 + row=3, column=3, sticky="ew", padx=(8, 0), pady=4 ) self.lbl_max_cand = ttk.Label(form, text=self._t("max_candidates"), style="Card.TLabel") - self.lbl_max_cand.grid(row=4, column=0, sticky="w", pady=5) + self.lbl_max_cand.grid(row=4, column=0, sticky="w", pady=4) self._register_text(self.lbl_max_cand, "max_candidates") - ttk.Entry(form, textvariable=self.mp_limit).grid(row=4, column=1, sticky="ew", padx=(8, 12), pady=5) + ttk.Entry(form, textvariable=self.mp_limit).grid(row=4, column=1, sticky="ew", padx=(8, 12), pady=4) self.chk_deprecated = ttk.Checkbutton( form, text=self._t("include_deprecated"), variable=self.mp_include_deprecated ) @@ -543,46 +656,48 @@ def _build_mp_tab(self, frame: ttk.Frame) -> None: form.columnconfigure(1, weight=1) form.columnconfigure(3, weight=1) - output_box = self._labeled_frame(left, "result_bundle", padding=10) - output_box.pack(fill="x", pady=(12, 0)) + output_box = self._labeled_frame(left, "result_bundle", padding=8) + output_box.pack(fill="x", pady=(10, 0)) self._path_entry(output_box, self.mp_output, self._choose_mp_output) self.chk_conventional = ttk.Checkbutton( output_box, text=self._t("conventional_cells"), variable=self.mp_conventional ) - self.chk_conventional.pack(anchor="w", pady=(8, 0)) + self.chk_conventional.pack(anchor="w", pady=(6, 0)) self._register_text(self.chk_conventional, "conventional_cells") self.chk_overwrite_mp = ttk.Checkbutton( output_box, text=self._t("overwrite_bundle"), variable=self.overwrite ) - self.chk_overwrite_mp.pack(anchor="w", pady=(4, 0)) + self.chk_overwrite_mp.pack(anchor="w", pady=(2, 0)) self._register_text(self.chk_overwrite_mp, "overwrite_bundle") self.mp_key_hint = ttk.Label( left, text=self._t("mp_key_hint"), style="Hint.TLabel", - wraplength=480, + wraplength=400, ) - self.mp_key_hint.pack(anchor="w", pady=(10, 0)) + self.mp_key_hint.pack(anchor="w", pady=(8, 4)) self._register_text(self.mp_key_hint, "mp_key_hint") + self._wrap_labels.append((self.mp_key_hint, 400)) self._card_title(right, "mp_analyze_title", "mp_analyze_hint") self._analysis_controls(right) - options = self._labeled_frame(right, "outputs", padding=10) - options.pack(fill="x", pady=(10, 0)) + options = self._labeled_frame(right, "outputs", padding=8) + options.pack(fill="x", pady=(8, 0)) self.chk_elasticity_mp = ttk.Checkbutton( options, text=self._t("eval_elasticity"), variable=self.include_elasticity ) - self.chk_elasticity_mp.pack(side="left", padx=(0, 14)) + self.chk_elasticity_mp.pack(anchor="w") self._register_text(self.chk_elasticity_mp, "eval_elasticity") self.chk_excel_mp = ttk.Checkbutton( options, text=self._t("write_excel"), variable=self.include_excel ) - self.chk_excel_mp.pack(side="left") + self.chk_excel_mp.pack(anchor="w") self._register_text(self.chk_excel_mp, "write_excel") + button = ttk.Button( - right, text=self._t("run_mp"), style="Primary.TButton", command=self._run_mp + footer, text=self._t("run_mp"), style="Primary.TButton", command=self._run_mp ) - button.pack(fill="x", pady=(14, 0)) + button.pack(fill="x") self._register_text(button, "run_mp") self._run_buttons.append(button) @@ -689,7 +804,7 @@ def _analysis_controls(self, parent: ttk.Frame) -> None: limits.columnconfigure(3, weight=1) def _build_cij_panel(self, parent: ttk.Frame) -> None: - box = self._labeled_frame(parent, "cij_panel", padding=10) + box = self._labeled_frame(parent, "cij_panel", padding=8) box.pack(fill="x", pady=(8, 0)) cubic = ttk.Frame(box, style="Card.TFrame") @@ -698,26 +813,37 @@ def _build_cij_panel(self, parent: ttk.Frame) -> None: lbl = ttk.Label(cubic, text=self._t(key), style="Card.TLabel") lbl.pack(side="left") self._register_text(lbl, key) - ttk.Entry(cubic, textvariable=var, width=8).pack(side="left", padx=(4, 10)) + ttk.Entry(cubic, textvariable=var, width=7).pack(side="left", padx=(4, 8)) btn_cubic = ttk.Button( cubic, text=self._t("apply_cubic"), style="Secondary.TButton", command=self._apply_cubic_cij ) btn_cubic.pack(side="left") self._register_text(btn_cubic, "apply_cubic") - paste_lbl = ttk.Label(box, text=self._t("cij_paste_hint"), style="Hint.TLabel") + paste_lbl = ttk.Label(box, text=self._t("cij_paste_hint"), style="Hint.TLabel", wraplength=400) paste_lbl.pack(anchor="w", pady=(8, 2)) self._register_text(paste_lbl, "cij_paste_hint") + self._wrap_labels.append((paste_lbl, 400)) + + paste_frame = ttk.Frame(box, style="Card.TFrame") + paste_frame.pack(fill="x") self.cij_paste = tk.Text( - box, - height=4, + paste_frame, + height=3, wrap="none", font=("Cascadia Mono", 8), relief="solid", borderwidth=1, highlightthickness=0, ) - self.cij_paste.pack(fill="x") + yscroll = ttk.Scrollbar(paste_frame, orient="vertical", command=self.cij_paste.yview) + xscroll = ttk.Scrollbar(paste_frame, orient="horizontal", command=self.cij_paste.xview) + self.cij_paste.configure(yscrollcommand=yscroll.set, xscrollcommand=xscroll.set) + self.cij_paste.grid(row=0, column=0, sticky="nsew") + yscroll.grid(row=0, column=1, sticky="ns") + xscroll.grid(row=1, column=0, sticky="ew") + paste_frame.columnconfigure(0, weight=1) + paste_frame.rowconfigure(0, weight=1) actions = ttk.Frame(box, style="Card.TFrame") actions.pack(fill="x", pady=(6, 0)) @@ -731,9 +857,9 @@ def _build_cij_panel(self, parent: ttk.Frame) -> None: ) btn_clear.pack(side="left", padx=(8, 0)) self._register_text(btn_clear, "clear_cij") - ttk.Label(box, textvariable=self.cij_status, style="Hint.TLabel", wraplength=420).pack( - anchor="w", pady=(6, 0) - ) + status = ttk.Label(box, textvariable=self.cij_status, style="Hint.TLabel", wraplength=400) + status.pack(anchor="w", pady=(6, 0)) + self._wrap_labels.append((status, 400)) def _path_entry(self, parent: Any, variable: Any, command: Callable[[], None]) -> None: row = ttk.Frame(parent, style="Card.TFrame") @@ -743,9 +869,10 @@ def _path_entry(self, parent: Any, variable: Any, command: Callable[[], None]) - btn.pack(side="left", padx=(8, 0)) self._register_text(btn, "browse") - def _build_activity_panel(self) -> None: - panel = ttk.Frame(self, padding=(18, 4, 18, 0)) - panel.pack(fill="x", side="bottom") + def _build_activity_panel(self, parent: Any | None = None) -> None: + host = parent if parent is not None else self + panel = ttk.Frame(host) + panel.pack(fill="both", expand=True) title_row = ttk.Frame(panel) title_row.pack(fill="x") self.activity_label = ttk.Label( @@ -764,10 +891,10 @@ def _build_activity_panel(self) -> None: btn_clear.pack(side="right") self._register_text(btn_clear, "clear_log") log_frame = tk.Frame(panel, bg=LOG_BG, highlightbackground=BORDER, highlightthickness=1) - log_frame.pack(fill="both", expand=True, pady=(5, 6)) + log_frame.pack(fill="both", expand=True, pady=(4, 4)) self.log = tk.Text( log_frame, - height=4, + height=5, wrap="word", state="disabled", bg=LOG_BG, @@ -786,16 +913,30 @@ def _build_activity_panel(self) -> None: self.log.tag_configure("success", foreground="#7CE3A1") self.log.tag_configure("warning", foreground="#FFD166") self.log.tag_configure("error", foreground="#FF8F88") + # Mouse wheel over the log scrolls the log itself. + def _log_wheel(event: Any) -> str | None: + delta = int(getattr(event, "delta", 0) or 0) + if delta: + self.log.yview_scroll(int(-delta / 120), "units") + elif getattr(event, "num", None) == 4: + self.log.yview_scroll(-3, "units") + elif getattr(event, "num", None) == 5: + self.log.yview_scroll(3, "units") + return "break" + + self.log.bind("", _log_wheel) + self.log.bind("", _log_wheel) + self.log.bind("", _log_wheel) def _build_status_bar(self) -> None: - bar = tk.Frame(self, bg="#E5EDF3", height=38) + bar = tk.Frame(self, bg="#E5EDF3", height=34) bar.pack(fill="x", side="bottom") bar.pack_propagate(False) ttk.Label(bar, textvariable=self.status_text, background="#E5EDF3", foreground=NAVY).pack( - side="left", padx=18 + side="left", padx=14 ) - self.progress = ttk.Progressbar(bar, mode="indeterminate", length=170) - self.progress.pack(side="right", padx=(8, 18), pady=9) + self.progress = ttk.Progressbar(bar, mode="indeterminate", length=140) + self.progress.pack(side="right", padx=(8, 14), pady=7) self.open_button = ttk.Button( bar, text=self._t("open_result"), @@ -803,7 +944,7 @@ def _build_status_bar(self) -> None: command=self._open_last_output, state="disabled", ) - self.open_button.pack(side="right", pady=4) + self.open_button.pack(side="right", pady=3) self._register_text(self.open_button, "open_result") def _enable_dnd(self, widget: Any) -> None: @@ -859,6 +1000,29 @@ def _apply_language(self) -> None: self.status_text.set(self._t("status_ready")) self._refresh_inputs() self._refresh_cij_status() + self.after_idle(self._update_wraplengths) + + def _on_root_configure(self, event: Any) -> None: + if event.widget is not self: + return + # Throttle wraplength updates while resizing. + if getattr(self, "_wrap_after_id", None) is not None: + try: + self.after_cancel(self._wrap_after_id) + except Exception: + pass + self._wrap_after_id = self.after(120, self._update_wraplengths) + + def _update_wraplengths(self) -> None: + self._wrap_after_id = None + width = max(int(self.winfo_width()), 400) + # Rough half-column width for two-column forms. + target = max(220, min(520, width // 2 - 80)) + for widget, _default in self._wrap_labels: + try: + widget.configure(wraplength=target) + except tk.TclError: + continue def _on_energy_shortcut(self) -> None: if self._syncing_shortcut: @@ -937,6 +1101,8 @@ def _apply_cubic_cij(self) -> None: _required_float(self.cij_c44.get(), "C44"), source="gui_cubic", ) + if tensor.status == "invalid": + raise ValueError(" | ".join(tensor.warnings)) except ValueError as exc: messagebox.showerror(self._t("err_cij_apply"), str(exc)) return @@ -953,6 +1119,8 @@ def _apply_matrix_cij(self) -> None: try: matrix = parse_cij_paste_text(self.cij_paste.get("1.0", "end")) tensor = parse_cij_matrix_6x6(matrix) + if tensor.status == "invalid": + raise ValueError(" | ".join(tensor.warnings)) except ValueError as exc: messagebox.showerror(self._t("err_cij_apply"), str(exc)) return @@ -1077,15 +1245,18 @@ def _run_local(self) -> None: return inputs = [str(path) for path in self.local_inputs] overrides = dict(self.elastic_overrides) if self.elastic_overrides else None + recursive = bool(self.local_recursive.get()) + include_excel = bool(self.include_excel.get()) + overwrite = bool(self.overwrite.get()) self._start_task( "Analyzing local CIF structures", lambda: analyze_cifs( inputs, output, settings=settings, - recursive=self.local_recursive.get(), - include_excel=self.include_excel.get(), - overwrite=self.overwrite.get(), + recursive=recursive, + include_excel=include_excel, + overwrite=overwrite, elastic_overrides=overrides, ), ) @@ -1111,6 +1282,10 @@ def _run_mp(self) -> None: messagebox.showerror(self._t("err_title_settings"), str(exc)) return + conventional = bool(self.mp_conventional.get()) + include_excel = bool(self.include_excel.get()) + overwrite = bool(self.overwrite.get()) + def run() -> PipelineResult: provider = MaterialsProjectProvider(api_key) return run_pipeline( @@ -1119,10 +1294,10 @@ def run() -> PipelineResult: output, discovery_settings=discovery, analysis_settings=analysis, - conventional_unit_cell=self.mp_conventional.get(), + conventional_unit_cell=conventional, include_elasticity=analysis.include_elasticity, - include_excel=self.include_excel.get(), - overwrite=self.overwrite.get(), + include_excel=include_excel, + overwrite=overwrite, confirm_above=limit, authorize_large_download=True, ) diff --git a/src/diffractscout/models.py b/src/diffractscout/models.py index d4a3d50..69b66aa 100644 --- a/src/diffractscout/models.py +++ b/src/diffractscout/models.py @@ -170,7 +170,7 @@ class ReflectionRecord: elastic_note: str = "" # Optional parity / export enrichment fields (defaults preserve lean callers). i: int | None = None # Miller–Bravais i = -(h+k) for hex/trigonal - two_theta_cu_ka_deg: float = 0.0 + two_theta_cu_ka_deg: float | None = None inverse_R_hkl: float | None = None inverse_R_hkl_no_lp: float | None = None phase_relative_R_hkl_pct: float = 0.0 diff --git a/src/diffractscout/pipeline.py b/src/diffractscout/pipeline.py index 0cf701d..11aec9b 100644 --- a/src/diffractscout/pipeline.py +++ b/src/diffractscout/pipeline.py @@ -11,7 +11,7 @@ from uuid import uuid4 from .composition import parse_composition_text -from .diffraction import simulate_powder_pattern +from .diffraction import simulate_powder_pattern, validate_analysis_settings from .elasticity import discover_elastic_tensor, validate_elastic_tensor from .exporters import export_result_bundle from .models import ( @@ -260,12 +260,16 @@ def _diagnostic_warnings(diagnostics: list[DiagnosticRecord]) -> list[str]: ) -def _verify_and_commit(target: Path, staging: Path) -> Path: +def _verify_and_commit(target: Path, staging: Path, *, overwrite: bool) -> Path: report = verify_bundle(staging) if not report["ok"]: raise RuntimeError( "Generated bundle failed its integrity check: " + "; ".join(report["errors"]) ) + # Close the long-running transaction's time-of-check/time-of-use gap. A + # target created or modified during analysis must satisfy the same overwrite + # policy as it did before the run started. + _validate_output_target(target, overwrite=overwrite) _commit_staging_output(target, staging) return target / "manifest.json" @@ -281,6 +285,7 @@ def analyze_cifs( elastic_overrides: Mapping[str, ElasticTensor] | None = None, ) -> PipelineResult: settings = settings or AnalysisSettings() + validate_analysis_settings(settings) _validate_input_output_separation(inputs, output_dir) paths = collect_cif_paths(inputs, recursive=recursive) if not paths: @@ -312,7 +317,7 @@ def analyze_cifs( diagnostics=diagnostics, include_excel=include_excel, ) - manifest = _verify_and_commit(target, staging) + manifest = _verify_and_commit(target, staging, overwrite=overwrite) except Exception: shutil.rmtree(staging, ignore_errors=True) raise @@ -346,12 +351,12 @@ def export_discovery( include_excel: bool = True, overwrite: bool = False, ) -> PipelineResult: + target = _validate_output_target(output_dir, overwrite=overwrite) discovery = discover_candidates( composition, provider, settings=discovery_settings, ) - target = _validate_output_target(output_dir, overwrite=overwrite) staging = _create_staging_output(target) diagnostics = [ DiagnosticRecord("discovery", "query", "warning", warning) @@ -367,7 +372,7 @@ def export_discovery( diagnostics=diagnostics, include_excel=include_excel, ) - manifest = _verify_and_commit(target, staging) + manifest = _verify_and_commit(target, staging, overwrite=overwrite) except Exception: shutil.rmtree(staging, ignore_errors=True) raise @@ -396,6 +401,14 @@ def run_pipeline( confirm_above: int = 200, authorize_large_download: bool = False, ) -> PipelineResult: + requested_analysis_settings = analysis_settings or AnalysisSettings( + include_elasticity=include_elasticity + ) + if requested_analysis_settings.include_elasticity != include_elasticity: + requested_analysis_settings = replace( + requested_analysis_settings, include_elasticity=include_elasticity + ) + validate_analysis_settings(requested_analysis_settings) if include_elasticity and not conventional_unit_cell: raise ValueError( "Primitive-cell downloads cannot be paired automatically with Materials Project " @@ -404,6 +417,7 @@ def run_pipeline( if isinstance(confirm_above, bool) or not isinstance(confirm_above, int) or confirm_above < 1: raise ValueError("confirm_above must be a positive integer.") + target = _validate_output_target(output_dir, overwrite=overwrite) discovery = discover_candidates( composition, provider, @@ -415,7 +429,6 @@ def run_pipeline( "without explicit authorization. Set authorize_large_download=True or reduce the query." ) - target = _validate_output_target(output_dir, overwrite=overwrite) staging = _create_staging_output(target) diagnostics = [ DiagnosticRecord("discovery", "query", "warning", warning) @@ -429,11 +442,7 @@ def run_pipeline( conventional_unit_cell=conventional_unit_cell, include_elasticity=include_elasticity, ) - settings = analysis_settings or AnalysisSettings( - include_elasticity=include_elasticity - ) - if settings.include_elasticity != include_elasticity: - settings = replace(settings, include_elasticity=include_elasticity) + settings = requested_analysis_settings path_pairs: list[tuple[Path, ElasticTensor | None]] = [] for item in downloads: @@ -486,7 +495,7 @@ def run_pipeline( diagnostics=diagnostics, include_excel=include_excel, ) - manifest = _verify_and_commit(target, staging) + manifest = _verify_and_commit(target, staging, overwrite=overwrite) except Exception: shutil.rmtree(staging, ignore_errors=True) raise diff --git a/src/diffractscout/providers/materials_project.py b/src/diffractscout/providers/materials_project.py index 7f8ab4f..89420a6 100644 --- a/src/diffractscout/providers/materials_project.py +++ b/src/diffractscout/providers/materials_project.py @@ -2,6 +2,7 @@ from __future__ import annotations +import math from pathlib import Path import tempfile from typing import Any, Sequence @@ -52,14 +53,17 @@ def _plain(value: object) -> object: def _matrix(value: object) -> list[list[float]] | None: plain = _plain(value) - if not isinstance(plain, list) or len(plain) < 6: + if not isinstance(plain, list) or len(plain) != 6: return None output: list[list[float]] = [] try: - for row in plain[:6]: - if not isinstance(row, list) or len(row) < 6: + for row in plain: + if not isinstance(row, list) or len(row) != 6: return None - output.append([float(item) for item in row[:6]]) + converted = [float(item) for item in row] + if not all(math.isfinite(item) for item in converted): + return None + output.append(converted) except (TypeError, ValueError): return None return output @@ -86,6 +90,8 @@ def _candidate_from_doc(doc: object, chemsys: str) -> CandidateRecord: e_hull = float(e_hull_raw) if e_hull_raw is not None else None except (TypeError, ValueError): e_hull = None + if e_hull is not None and not math.isfinite(e_hull): + e_hull = None structure_type = infer_structure_type(formula, symbol, number) return CandidateRecord( material_id=material_id, diff --git a/src/diffractscout/quick_export.py b/src/diffractscout/quick_export.py index 5c73bb6..e5c78b8 100644 --- a/src/diffractscout/quick_export.py +++ b/src/diffractscout/quick_export.py @@ -9,6 +9,8 @@ import argparse import shutil import sys +import tempfile +from dataclasses import replace from pathlib import Path from typing import Mapping, Sequence @@ -20,6 +22,43 @@ _SETTINGS_KEYS = frozenset(AnalysisSettings.__dataclass_fields__) +def _validate_excel_target(path: Path, *, overwrite: bool) -> None: + """Protect a user-selected workbook before the bundle run starts.""" + + if path.is_symlink(): + raise FileExistsError(f"Refusing to replace a symbolic-link Excel target: {path}") + if not path.exists(): + return + if not path.is_file(): + raise FileExistsError(f"Excel output exists and is not a file: {path}") + if not overwrite: + raise FileExistsError( + f"Excel output already exists: {path}. Choose a new path or pass overwrite=True." + ) + + +def _copy_excel_atomic(source: Path, target: Path, *, overwrite: bool) -> None: + """Copy a completed bundle workbook without exposing a partial target.""" + + _validate_excel_target(target, overwrite=overwrite) + target.parent.mkdir(parents=True, exist_ok=True) + temporary_name = "" + try: + with tempfile.NamedTemporaryFile( + prefix=f".{target.name}.", + suffix=".tmp", + dir=target.parent, + delete=False, + ) as handle: + temporary_name = handle.name + temporary = Path(temporary_name) + shutil.copy2(source, temporary) + temporary.replace(target) + finally: + if temporary_name: + Path(temporary_name).unlink(missing_ok=True) + + def _default_settings(**overrides: object) -> AnalysisSettings: """Cu Kα, 5–120°, lab views on; other fields match AnalysisSettings defaults.""" @@ -73,6 +112,7 @@ def quick_export( "Unexpected keyword arguments for quick_export: " + ", ".join(sorted(map(str, leftover))) ) + settings = replace(settings, **kwargs) if elastic_overrides is not None and not isinstance(elastic_overrides, Mapping): raise TypeError("elastic_overrides must be a mapping of name -> ElasticTensor.") @@ -85,6 +125,7 @@ def quick_export( excel_target: Path | None = None if output_path.suffix.lower() == ".xlsx": excel_target = output_path if output_path.is_absolute() else output_path.resolve() + _validate_excel_target(excel_target, overwrite=overwrite) bundle_dir = excel_target.with_name(f"{excel_target.stem}_bundle") # Excel shortcut always materializes the workbook in the bundle first. include_excel = True @@ -107,8 +148,7 @@ def quick_export( raise RuntimeError( f"Expected results.xlsx in bundle {result.output_dir}, but it is missing." ) - excel_target.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(source_xlsx, excel_target) + _copy_excel_atomic(source_xlsx, excel_target, overwrite=overwrite) return result diff --git a/src/diffractscout/selection.py b/src/diffractscout/selection.py index 6fe22ab..f1b55e5 100644 --- a/src/diffractscout/selection.py +++ b/src/diffractscout/selection.py @@ -13,6 +13,13 @@ def validate_discovery_settings(settings: DiscoverySettings) -> None: """Reject ambiguous or unsafe discovery limits before provider access.""" + valid_modes = {"possible_phases", "near_stable", "single_chemsys", "mpids_only"} + if settings.mode not in valid_modes: + raise ValueError( + f"Unknown discovery mode {settings.mode!r}; choose one of: " + + ", ".join(sorted(valid_modes)) + + "." + ) if settings.e_hull_max_eV_atom is not None: value = float(settings.e_hull_max_eV_atom) if not math.isfinite(value) or value < 0: diff --git a/src/diffractscout/structure.py b/src/diffractscout/structure.py index 02365be..b3fd358 100644 --- a/src/diffractscout/structure.py +++ b/src/diffractscout/structure.py @@ -4,6 +4,7 @@ import math import re +import warnings as warning_control from collections import defaultdict from pathlib import Path from typing import Iterable @@ -217,7 +218,22 @@ def _spglib_crosscheck( lattice = np.asarray(small.cell.orth.mat, dtype=float).T positions = np.asarray([[site.fract.x, site.fract.y, site.fract.z] for site in sites], dtype=float) atomic_numbers = np.asarray([site.element.atomic_number for site in sites], dtype=int) - dataset = spglib.get_symmetry_dataset((lattice, positions, atomic_numbers), symprec=1e-3) + try: + with warning_control.catch_warnings(): + # spglib 2.7 warns before its 2.8 exception-mode transition. The + # cross-check already handles failure explicitly, so suppress only + # that upstream compatibility warning and keep all other warnings. + warning_control.filterwarnings( + "ignore", + message="Set OLD_ERROR_HANDLING to false and catch the errors directly.", + category=DeprecationWarning, + module=r"spglib(?:\..*)?", + ) + dataset = spglib.get_symmetry_dataset( + (lattice, positions, atomic_numbers), symprec=1e-3 + ) + except Exception: + return None, None, "failed" if dataset is None: return None, None, "failed" number = int(dataset.number) diff --git a/tests/test_composition.py b/tests/test_composition.py index e2a988f..4a8a339 100644 --- a/tests/test_composition.py +++ b/tests/test_composition.py @@ -14,6 +14,17 @@ def test_formula_and_mpids_can_coexist() -> None: assert parsed.material_ids == ("mp-23", "mp-149") +def test_common_unicode_dashes_and_lowercase_chemsys_are_normalized() -> None: + alloy = parse_composition_text("Ti–10V–2Fe–3Al") + assert set(alloy.elements) == {"Ti", "V", "Fe", "Al"} + + chemsys = parse_composition_text("ti-al-v") + assert set(chemsys.elements) == {"Ti", "Al", "V"} + + material = parse_composition_text("MP‑149") + assert material.material_ids == ("mp-149",) + + def test_subsystem_order_limit() -> None: systems = chemsys_subsystems(["Ti", "Al", "V", "Cu"], max_order=2) assert len(systems) == 10 diff --git a/tests/test_export_views.py b/tests/test_export_views.py index c4bc0de..78627c4 100644 --- a/tests/test_export_views.py +++ b/tests/test_export_views.py @@ -8,7 +8,13 @@ from openpyxl import load_workbook from diffractscout.demo import write_demo_inputs -from diffractscout.export_views import BEGINNER_PEAK_HEADERS_ZH, beginner_peak_rows_zh, user_guide_rows +from diffractscout.export_views import ( + ANALYSIS_PEAK_COLUMNS, + BEGINNER_PEAK_HEADERS_ZH, + beginner_peak_rows_zh, + user_guide_rows, +) +from diffractscout.exporters import PEAK_HEADERS, peak_rows, write_excel_workbook from diffractscout.models import AnalysisSettings from diffractscout.pipeline import analyze_cifs @@ -23,18 +29,64 @@ def test_demo_analyze_writes_lab_view_sheets(tmp_path: Path) -> None: ) workbook_path = result.output_dir / "results.xlsx" assert workbook_path.is_file() - workbook = load_workbook(workbook_path, data_only=False, read_only=True) + workbook = load_workbook(workbook_path, data_only=False, read_only=False) assert "推荐峰表" in workbook.sheetnames assert "使用说明" in workbook.sheetnames assert "Peaks" in workbook.sheetnames + # Lab-first workbook opens on the Chinese analysis long table. + assert workbook.active.title == "推荐峰表" recommend = workbook["推荐峰表"] headers = [cell.value for cell in next(recommend.iter_rows(min_row=1, max_row=1))] assert headers[0] == "物相名称" - assert "相对强度" in headers + assert "相对强度_相内max100" in headers + assert "2θ_当前_deg" in headers + assert "2θ_CuKa_deg" in headers + assert "体积归一强度J_含LP_R_hkl别名" in headers guide = workbook["使用说明"] guide_rows = list(guide.iter_rows(min_row=1, max_row=3, values_only=True)) assert guide_rows[0][0] == "项目" - assert any("R_hkl" in str(row[0] or "") or "R_hkl" in str(row[1] or "") for row in guide.iter_rows(values_only=True)) + joined = "\n".join( + str(cell or "") for row in guide.iter_rows(values_only=True) for cell in row + ) + assert "残差" in joined + assert "不是" in joined + + +def test_peak_and_recommend_sheets_have_freeze_and_autofilter(tmp_path: Path) -> None: + inputs = write_demo_inputs(tmp_path / "inputs") + result = analyze_cifs( + [inputs], + tmp_path / "bundle", + settings=AnalysisSettings(export_lab_views=True, include_elasticity=True), + include_excel=True, + ) + workbook = load_workbook(result.output_dir / "results.xlsx", data_only=False) + for sheet_name in ("Peaks", "推荐峰表"): + sheet = workbook[sheet_name] + assert sheet.freeze_panes == "A2" + assert sheet.auto_filter.ref + assert sheet.auto_filter.ref.startswith("A1:") + headers = [cell.value for cell in next(sheet.iter_rows(min_row=1, max_row=1))] + assert headers[0] in {"phase_name", "物相名称"} + assert sheet.max_row >= 2 + + +def test_analysis_peak_columns_present_in_peaks_and_csv(tmp_path: Path) -> None: + inputs = write_demo_inputs(tmp_path / "inputs") + result = analyze_cifs([inputs], tmp_path / "bundle", include_excel=True) + for col in ANALYSIS_PEAK_COLUMNS: + assert col in PEAK_HEADERS + workbook = load_workbook(result.output_dir / "results.xlsx", data_only=False) + peak_headers = [cell.value for cell in next(workbook["Peaks"].iter_rows(min_row=1, max_row=1))] + for col in ANALYSIS_PEAK_COLUMNS: + assert col in peak_headers + with (result.output_dir / "peak_reference.csv").open(encoding="utf-8-sig", newline="") as handle: + fieldnames = list(csv.DictReader(handle).fieldnames or []) + for col in ANALYSIS_PEAK_COLUMNS: + assert col in fieldnames + # Analysis-first ordering: identity and geometry before deep SF extras. + assert peak_headers.index("phase_name") < peak_headers.index("d_spacing_A") + assert peak_headers.index("normalized_intensity") < peak_headers.index("structure_factor_sq") def test_peak_reference_has_two_theta_cu_ka_column(tmp_path: Path) -> None: @@ -105,6 +157,40 @@ def test_export_lab_views_false_omits_chinese_sheets(tmp_path: Path) -> None: assert "Peaks" in workbook.sheetnames +def test_multi_phase_peak_rows_keep_phase_name_for_filter(tmp_path: Path) -> None: + """Shipped peak_rows + Excel Peaks support multi-phase filter via phase_name.""" + + demo_dir = write_demo_inputs(tmp_path / "inputs") + # Second synthetic phase: copy the demo CIF under another stem for multi-phase. + src = next(demo_dir.glob("*.cif")) + second = demo_dir / "phase_b.cif" + second.write_bytes(src.read_bytes()) + # Pair elasticity only for first if present; analysis still works. + result = analyze_cifs( + [demo_dir], + tmp_path / "bundle", + settings=AnalysisSettings(export_lab_views=True, include_elasticity=False), + include_excel=True, + ) + assert len(result.analyses) >= 2 + rows = peak_rows(result.analyses) + phases = {row["phase_name"] for row in rows} + assert len(phases) >= 2 + workbook = load_workbook(result.output_dir / "results.xlsx", data_only=False) + peak_headers = [cell.value for cell in next(workbook["Peaks"].iter_rows(min_row=1, max_row=1))] + assert peak_headers[0] == "phase_name" + rec_headers = [cell.value for cell in next(workbook["推荐峰表"].iter_rows(min_row=1, max_row=1))] + assert rec_headers[0] == "物相名称" + # All peak data rows carry a non-empty phase identity for Excel filter. + for sheet_name, key in (("Peaks", "phase_name"), ("推荐峰表", "物相名称")): + sheet = workbook[sheet_name] + headers = [cell.value for cell in next(sheet.iter_rows(min_row=1, max_row=1))] + col = headers.index(key) + 1 + values = [sheet.cell(row=r, column=col).value for r in range(2, sheet.max_row + 1)] + assert values + assert all(v not in (None, "") for v in values) + + def test_beginner_and_guide_helpers_are_stable() -> None: assert BEGINNER_PEAK_HEADERS_ZH["物相名称"] == "phase_name" assert BEGINNER_PEAK_HEADERS_ZH["2θ_CuKa_deg"] == "two_theta_cu_ka_deg" @@ -112,9 +198,54 @@ def test_beginner_and_guide_helpers_are_stable() -> None: [{"phase_name": "Al", "normalized_intensity": 100.0, "two_theta_cu_ka_deg": 38.0}] ) assert mapped[0]["物相名称"] == "Al" - assert mapped[0]["相对强度"] == 100.0 + assert mapped[0]["相对强度_相内max100"] == 100.0 guide = user_guide_rows() assert guide[0] == ["项目", "说明"] joined = "\n".join(cell for row in guide for cell in row) assert "不是" in joined and "残差" in joined assert "QPA" in joined + assert "筛选物相" in joined + + +def test_user_guide_rank_and_theta_wording_is_consistent() -> None: + """Guide must match rank_by_*=1 strongest and 2θ=2×θ (shipped user_guide_rows).""" + + guide = user_guide_rows() + # θ explanation must state double-angle relation, not tautology. + theta_rows = [row for row in guide if row and "theta_deg" in str(row[0])] + assert theta_rows + assert "2×θ" in theta_rows[0][1] or "2*θ" in theta_rows[0][1] + assert "2θ = 2θ" not in theta_rows[0][1] + # Rank: 1 is strongest → smaller rank is stronger. + rank_rows = [row for row in guide if row and "rank_by" in str(row[0])] + assert rank_rows + assert "1 为最强" in rank_rows[0][1] or "1为最强" in rank_rows[0][1] + assert "数值越大排名越靠前" not in rank_rows[0][1] + # How-to: strong peaks via relative intensity DESC or rank ASC / filter 1..N + tip_rows = [row for row in guide if row and str(row[0]) == "找强峰"] + assert tip_rows + tip = tip_rows[0][1] + assert "相对强度" in tip and "降序" in tip + assert "rank_by_intensity 降序" not in tip + assert "1..5" in tip or "1–5" in tip + + +def test_write_excel_workbook_freeze_filter_on_empty_peaks(tmp_path: Path) -> None: + path = tmp_path / "empty.xlsx" + write_excel_workbook( + path, + summary=[{"key": "k", "value": "v"}], + phases=[], + peaks=[], + elasticity=[], + candidates=[], + downloads=[], + diagnostics=[], + patterns=[], + export_lab_views=True, + include_patterns=False, + ) + workbook = load_workbook(path) + peaks = workbook["Peaks"] + assert peaks.freeze_panes == "A2" + assert peaks.auto_filter.ref diff --git a/tests/test_gui.py b/tests/test_gui.py index b5e5eb2..a2a7d7e 100644 --- a/tests/test_gui.py +++ b/tests/test_gui.py @@ -1,7 +1,9 @@ import pytest -from diffractscout.gui import analysis_settings_from_form, discovery_settings_from_form +import diffractscout.gui as gui_module +from diffractscout.gui import analysis_settings_from_form, create_app, discovery_settings_from_form from diffractscout.gui_i18n import REQUIRED_KEYS, STRINGS, assert_language_parity, t +from diffractscout.models import AnalysisSettings def test_analysis_form_builds_energy_settings() -> None: @@ -74,12 +76,24 @@ def test_analysis_form_rejects_unknown_profile_model() -> None: ) +def _create_test_app(): + try: + return create_app() + except RuntimeError as exc: + if "Tkinter" in str(exc): + pytest.skip("Tkinter unavailable") + raise + except Exception as exc: + if gui_module.tk is not None and isinstance(exc, gui_module.tk.TclError): + pytest.skip(f"Tk display unavailable: {exc}") + raise + + def test_discovery_form_rejects_zero_limit() -> None: - settings = discovery_settings_from_form( - {"mode": "possible_phases", "max_total": "0"} - ) - assert settings.max_total == 0 - # Domain validation occurs before provider access in search_candidates. + with pytest.raises(ValueError, match="max_total"): + discovery_settings_from_form( + {"mode": "possible_phases", "max_total": "0"} + ) def test_analysis_form_rejects_non_numeric_value() -> None: @@ -109,3 +123,76 @@ def test_i18n_required_keys_zh_en_parity() -> None: assert t("en", key) assert t("zh", key) != key assert t("en", key) != key + + +def test_gui_layout_has_scrollable_regions_and_run_buttons() -> None: + """Smoke: app builds with scroll helpers and always-visible run buttons.""" + + app = _create_test_app() + try: + app.geometry("960x640") + app.update_idletasks() + assert hasattr(app, "_main_paned") + assert getattr(app, "_scroll_canvases", None) + assert len(app._scroll_canvases) >= 2 + assert len(app._run_buttons) >= 2 + for button in app._run_buttons: + assert str(button.winfo_manager()) in {"pack", "grid", "place"} + # Footer-pinned buttons should report a positive height after layout. + assert int(button.winfo_reqheight()) > 0 + finally: + app.destroy() + + +def test_local_worker_uses_ui_state_snapshot(tmp_path, monkeypatch) -> None: + """Worker options are frozen without needing a live Tk display.""" + + if not hasattr(gui_module.DiffractScoutApp, "_run_local"): + pytest.skip("Tkinter unavailable") + + class Variable: + def __init__(self, value): + self.value = value + + def get(self): + return self.value + + def set(self, value): + self.value = value + + class Controller: + pass + + controller = Controller() + sentinel = object() + observed: dict[str, object] = {} + scheduled: dict[str, object] = {} + + def fake_analyze(inputs, output, **kwargs): + observed.update({"inputs": inputs, "output": output, **kwargs}) + return sentinel + + monkeypatch.setattr(gui_module, "analyze_cifs", fake_analyze) + controller.running = False + controller.local_inputs = [tmp_path / "input.cif"] + controller.local_output = Variable(str(tmp_path / "bundle")) + controller.elastic_overrides = {} + controller.local_recursive = Variable(True) + controller.include_excel = Variable(False) + controller.overwrite = Variable(False) + controller._form_analysis_settings = lambda: AnalysisSettings() + controller._start_task = lambda label, function: scheduled.update( + {"label": label, "function": function} + ) + + gui_module.DiffractScoutApp._run_local(controller) + function = scheduled["function"] + controller.local_recursive.set(False) + controller.include_excel.set(True) + controller.overwrite.set(True) + + assert callable(function) + assert function() is sentinel + assert observed["recursive"] is True + assert observed["include_excel"] is False + assert observed["overwrite"] is False diff --git a/tests/test_parity_features.py b/tests/test_parity_features.py index 9d07ab2..5db5ccf 100644 --- a/tests/test_parity_features.py +++ b/tests/test_parity_features.py @@ -104,6 +104,25 @@ def test_two_theta_cu_ka_present_and_finite(demo_inputs: Path) -> None: assert result.metadata["density_g_cm3"] == pytest.approx(dens) +def test_cu_ka_convenience_angle_is_missing_when_reflection_is_inaccessible( + demo_inputs: Path, +) -> None: + structure = load_structure(demo_inputs / "synthetic_fcc_al.cif") + result = simulate_powder_pattern( + structure, + AnalysisSettings( + source_preset="Ag Ka", + two_theta_min_deg=5.0, + two_theta_max_deg=170.0, + ), + ) + inaccessible = [ + item for item in result.reflections if item.d_spacing_A < CU_KA_WAVELENGTH_A / 2.0 + ] + assert inaccessible + assert all(item.two_theta_cu_ka_deg is None for item in inaccessible) + + def test_hexagonal_miller_bravais_labeling(tmp_path: Path) -> None: cif_path = tmp_path / "synthetic_hex_mg.cif" cif_path.write_text(HEX_MG_CIF, encoding="utf-8") diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 57bb06d..e4a0772 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -3,9 +3,10 @@ import shutil from openpyxl import load_workbook +import pytest from diffractscout.models import AnalysisSettings -from diffractscout.pipeline import analyze_cifs, collect_cif_paths +from diffractscout.pipeline import analyze_cifs, collect_cif_paths, run_pipeline from diffractscout.validation import verify_bundle @@ -125,6 +126,40 @@ def test_input_output_overlap_is_rejected_before_writes(demo_inputs: Path) -> No assert not output.exists() +def test_invalid_run_settings_fail_before_output_writes( + demo_inputs: Path, tmp_path: Path +) -> None: + output = tmp_path / "invalid-settings" + with pytest.raises(ValueError, match="2theta range"): + analyze_cifs( + [demo_inputs], + output, + settings=AnalysisSettings( + two_theta_min_deg=120.0, + two_theta_max_deg=5.0, + ), + include_excel=False, + ) + assert not output.exists() + + +def test_invalid_figure_preset_fails_before_output_writes( + demo_inputs: Path, tmp_path: Path +) -> None: + output = tmp_path / "invalid-figure-preset" + with pytest.raises(ValueError, match="Unknown figure export preset"): + analyze_cifs( + [demo_inputs], + output, + settings=AnalysisSettings( + include_figures=True, + figure_preset="not-a-preset", + ), + include_excel=False, + ) + assert not output.exists() + + def test_workbook_contains_structured_diagnostics_sheet(demo_inputs: Path, tmp_path: Path) -> None: output = tmp_path / "diagnostic-workbook" analyze_cifs([demo_inputs], output) @@ -170,3 +205,47 @@ def fail_export(*_args, **_kwargs): raise AssertionError("Expected staged export failure") assert (output / "manifest.json").read_bytes() == original_manifest assert verify_bundle(output)["ok"] + + +def test_target_created_during_run_is_not_silently_replaced( + demo_inputs: Path, tmp_path: Path, monkeypatch +) -> None: + import diffractscout.pipeline as pipeline + + output = tmp_path / "raced-target" + original_export = pipeline.export_result_bundle + + def export_then_occupy(*args, **kwargs): + manifest = original_export(*args, **kwargs) + output.mkdir() + (output / "user.txt").write_text("keep", encoding="utf-8") + return manifest + + monkeypatch.setattr(pipeline, "export_result_bundle", export_then_occupy) + with pytest.raises(FileExistsError, match="not empty"): + analyze_cifs([demo_inputs], output, include_excel=False) + + assert (output / "user.txt").read_text(encoding="utf-8") == "keep" + assert not list(tmp_path.glob(".raced-target.diffractscout-*")) + + +def test_remote_provider_is_not_contacted_when_output_is_unsafe(tmp_path: Path) -> None: + class Provider: + name = "not-called" + + def search_subsystem(self, *_args, **_kwargs): + raise AssertionError("provider should not be contacted") + + def download_candidates(self, *_args, **_kwargs): + raise AssertionError("provider should not be contacted") + + def metadata(self): + raise AssertionError("provider should not be contacted") + + output = tmp_path / "occupied" + output.mkdir() + (output / "user.txt").write_text("keep", encoding="utf-8") + + with pytest.raises(FileExistsError, match="not empty"): + run_pipeline("Ti-Al", Provider(), output) + assert (output / "user.txt").read_text(encoding="utf-8") == "keep" diff --git a/tests/test_provider_helpers.py b/tests/test_provider_helpers.py index 8dcc7e3..51c1ee9 100644 --- a/tests/test_provider_helpers.py +++ b/tests/test_provider_helpers.py @@ -1,4 +1,5 @@ import json +import math from pathlib import Path from diffractscout.elasticity import MP_CONVENTIONAL_CIF_FRAME, MP_IEEE_CONVENTIONAL_FRAME @@ -49,6 +50,9 @@ def test_matrix_conversion() -> None: matrix = [[float(i == j) for j in range(6)] for i in range(6)] assert _matrix(matrix) == matrix assert _matrix([[1, 2]]) is None + assert _matrix([[float(i == j) for j in range(7)] for i in range(7)]) is None + matrix[0][0] = math.nan + assert _matrix(matrix) is None def test_elasticity_sidecar_prefers_raw_tensor_for_conventional_cif(tmp_path: Path) -> None: diff --git a/tests/test_quick_export.py b/tests/test_quick_export.py index 7ef93f2..bad8895 100644 --- a/tests/test_quick_export.py +++ b/tests/test_quick_export.py @@ -4,6 +4,7 @@ from openpyxl import load_workbook from diffractscout.elasticity_input import parse_cubic_cij +from diffractscout.models import AnalysisSettings from diffractscout.pipeline import analyze_cifs from diffractscout.quick_export import main as quick_export_main from diffractscout.quick_export import quick_export @@ -23,6 +24,32 @@ def test_quick_export_xlsx_writes_excel_and_bundle(demo_inputs: Path, tmp_path: assert "Peaks" in workbook.sheetnames +def test_quick_export_does_not_replace_existing_excel_without_authorization( + demo_inputs: Path, tmp_path: Path +) -> None: + excel = tmp_path / "existing.xlsx" + excel.write_bytes(b"user workbook") + + with pytest.raises(FileExistsError, match="already exists"): + quick_export([demo_inputs], excel) + + assert excel.read_bytes() == b"user workbook" + assert not (tmp_path / "existing_bundle").exists() + + +def test_quick_export_can_atomically_replace_existing_excel_when_authorized( + demo_inputs: Path, tmp_path: Path +) -> None: + excel = tmp_path / "existing.xlsx" + excel.write_bytes(b"old workbook") + + result = quick_export([demo_inputs], excel, overwrite=True) + + assert result.output_dir == (tmp_path / "existing_bundle").resolve() + assert excel.read_bytes() != b"old workbook" + assert load_workbook(excel, read_only=True).sheetnames + + def test_quick_export_directory_mode(demo_inputs: Path, tmp_path: Path) -> None: output = tmp_path / "lab_bundle" result = quick_export([demo_inputs], output) @@ -31,6 +58,19 @@ def test_quick_export_directory_mode(demo_inputs: Path, tmp_path: Path) -> None: assert result.analyses[0].metadata.get("export_lab_views") is True +def test_quick_export_keyword_overrides_apply_to_explicit_settings( + demo_inputs: Path, tmp_path: Path +) -> None: + result = quick_export( + [demo_inputs], + tmp_path / "overrides", + settings=AnalysisSettings(step_deg=0.02), + step_deg=0.05, + include_excel=False, + ) + assert result.analyses[0].metadata["step_deg"] == pytest.approx(0.05) + + def test_quick_export_cli_entry(demo_inputs: Path, tmp_path: Path) -> None: excel = tmp_path / "cli_out.xlsx" code = quick_export_main([str(demo_inputs), "-o", str(excel)]) diff --git a/tests/test_selection.py b/tests/test_selection.py index 1543ec7..4f9005e 100644 --- a/tests/test_selection.py +++ b/tests/test_selection.py @@ -79,6 +79,16 @@ def test_invalid_discovery_limits_fail_before_provider_access() -> None: raise AssertionError("Expected invalid max_total to be rejected") +def test_unknown_discovery_mode_fails_before_provider_access() -> None: + parsed = parse_composition_text("Ti-Al") + with pytest.raises(ValueError, match="Unknown discovery mode"): + search_candidates( + FakeProvider(), + parsed, + DiscoverySettings(mode="typo"), # type: ignore[arg-type] + ) + + def test_subsystem_expansion_limit_fails_before_provider_access() -> None: parsed = parse_composition_text("Ti-Al-V-Cu") with pytest.raises(ValueError, match="above max_subsystems=10"): diff --git a/tests/test_structure.py b/tests/test_structure.py index 9cd98f3..ce303ed 100644 --- a/tests/test_structure.py +++ b/tests/test_structure.py @@ -1,10 +1,14 @@ from pathlib import Path +import warnings from diffractscout.structure import load_structure def test_load_synthetic_fcc(demo_inputs: Path) -> None: - structure = load_structure(demo_inputs / "synthetic_fcc_al.cif") + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + structure = load_structure(demo_inputs / "synthetic_fcc_al.cif") + assert not any("OLD_ERROR_HANDLING" in str(item.message) for item in caught) assert structure.formula == "Al" assert structure.space_group_number == 225 assert structure.space_group_symbol.replace(" ", "") == "Fm-3m" From a513da58cc15a4b5f076da392657ea7da5a63204 Mon Sep 17 00:00:00 2001 From: Delun Gong Date: Wed, 12 Aug 2026 20:23:44 +0800 Subject: [PATCH 2/2] fix: apply MP e_hull filter before page limit Avoid dropping near-stable candidates when local post-filter is applied after a truncated first page. Reject unknown e_hull when a max is set. --- CHANGELOG.md | 1 + .../providers/materials_project.py | 14 +++--- tests/test_provider_helpers.py | 49 +++++++++++++++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 703152f..6b25bde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ All notable changes are recorded here. The project follows semantic versioning a - Required explicit overwrite authorization for existing quick-export workbooks and replaced authorized workbooks atomically. - Rechecked bundle and benchmark targets immediately before commit so files created during a long-running calculation are not silently replaced. - Rejected malformed or non-finite provider elasticity matrices instead of truncating oversized arrays to 6×6. +- Applied Materials Project stability thresholds before server-side result limits, with strict local verification so qualifying candidates are not lost to post-filtering. - Normalized common Unicode dash characters and case-insensitive chemical-system input during composition parsing. - Kept the optional spglib cross-check warning-only across its 2.7-to-2.8 exception transition without emitting repeated deprecation noise. - Exported an empty Cu Kα convenience angle, rather than a false `0°`, when a reflection is inaccessible at that wavelength. diff --git a/src/diffractscout/providers/materials_project.py b/src/diffractscout/providers/materials_project.py index 89420a6..a6caf2d 100644 --- a/src/diffractscout/providers/materials_project.py +++ b/src/diffractscout/providers/materials_project.py @@ -199,6 +199,10 @@ def search_subsystem( kwargs: dict[str, Any] = {"chemsys": chemsys, "fields": fields} if exclude_deprecated: kwargs["deprecated"] = False + if e_hull_max_eV_atom is not None: + # Filter before applying the page limit; otherwise a locally filtered + # first page can omit qualifying candidates from later pages. + kwargs["energy_above_hull"] = (0.0, e_hull_max_eV_atom) if max_results is not None and max_results > 0: kwargs.update({"chunk_size": max_results, "num_chunks": 1}) @@ -216,12 +220,10 @@ def search_subsystem( continue if exclude_deprecated and candidate.deprecated is True: continue - if ( - e_hull_max_eV_atom is not None - and candidate.energy_above_hull_eV_atom is not None - and candidate.energy_above_hull_eV_atom > e_hull_max_eV_atom - ): - continue + if e_hull_max_eV_atom is not None: + energy = candidate.energy_above_hull_eV_atom + if energy is None or energy > e_hull_max_eV_atom: + continue output.append(candidate) if max_results is not None and len(output) >= max_results: break diff --git a/tests/test_provider_helpers.py b/tests/test_provider_helpers.py index 51c1ee9..3256f88 100644 --- a/tests/test_provider_helpers.py +++ b/tests/test_provider_helpers.py @@ -55,6 +55,55 @@ def test_matrix_conversion() -> None: assert _matrix(matrix) is None +def test_subsystem_query_filters_energy_before_page_limit() -> None: + calls: list[dict[str, object]] = [] + + class SummaryEndpoint: + def search(self, **kwargs: object) -> list[dict[str, object]]: + calls.append(kwargs) + return [ + { + "material_id": "mp-1", + "formula_pretty": "Al", + "energy_above_hull": 0.02, + "deprecated": False, + }, + { + "material_id": "mp-unknown", + "formula_pretty": "Al", + "energy_above_hull": None, + "deprecated": False, + }, + ] + + class Materials: + summary = SummaryEndpoint() + + class Client: + materials = Materials() + + def __enter__(self) -> "Client": + return self + + def __exit__(self, *_args: object) -> None: + return None + + def get_database_version(self) -> str: + return "test" + + provider = _provider_without_client() + provider.api_key = "test" + provider._mpr_cls = lambda _api_key: Client() + provider._metadata = {} + + candidates = provider.search_subsystem("Al", max_results=5, e_hull_max_eV_atom=0.05) + + assert [candidate.material_id for candidate in candidates] == ["mp-1"] + assert calls[0]["energy_above_hull"] == (0.0, 0.05) + assert calls[0]["chunk_size"] == 5 + assert calls[0]["num_chunks"] == 1 + + def test_elasticity_sidecar_prefers_raw_tensor_for_conventional_cif(tmp_path: Path) -> None: cif_path = tmp_path / "mp-123_Al.cif" cif_path.write_text("data_test\n", encoding="utf-8")