diff --git a/pixi.lock b/pixi.lock index f45289f91..1d520d7da 100644 --- a/pixi.lock +++ b/pixi.lock @@ -9665,6 +9665,7 @@ packages: - diffpy-pdffit2 - diffpy-utils - emcee + - filelock - gemmi - h5py - lmfit diff --git a/pyproject.toml b/pyproject.toml index 50e2a410e..57c7c712f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,6 +26,7 @@ dependencies = [ 'numpy', # Numerical computing library 'asciichartpy', # ASCII charts for terminal output 'pooch', # Data downloader + 'filelock', # Cross-process locking for shared download caches 'typer', # Command-line interface creation 'rich', # Rich text and beautiful formatting in the terminal 'varname', # Variable name introspection diff --git a/src/easydiffraction/analysis/calculators/crysfml.py b/src/easydiffraction/analysis/calculators/crysfml.py index e3fae3c30..6520126b3 100644 --- a/src/easydiffraction/analysis/calculators/crysfml.py +++ b/src/easydiffraction/analysis/calculators/crysfml.py @@ -24,7 +24,7 @@ from __future__ import annotations -import string +import re from typing import TYPE_CHECKING import numpy as np @@ -60,23 +60,26 @@ def _element_symbol(type_symbol: str) -> str: """ - Strip a leading isotope number from an atom type symbol. + Extract the element from an isotope or ionic atom type symbol. CrysFML resolves scattering by element and does not understand - isotope prefixes such as ``11B`` or ``2H`` (cryspy does). Returning - the bare element symbol lets one model drive both engines. + isotope prefixes such as ``11B`` or ionic suffixes such as ``Fe3+`` + (cryspy does). Returning the bare element symbol lets one model + drive both engines. Parameters ---------- type_symbol : str - Atom type symbol, optionally isotope-prefixed (e.g. ``11B``). + Atom type symbol, optionally isotope-prefixed or charged (e.g. + ``11B`` or ``Fe3+``). Returns ------- str - The symbol with any leading digits removed (e.g. ``B``). + The bare element symbol (e.g. ``B`` or ``Fe``). """ - return type_symbol.lstrip(string.digits) + match = re.fullmatch(r'\d*([A-Z][a-z]?)(?:[1-8][+-])?', type_symbol.strip()) + return match.group(1) if match else type_symbol def _cfl_label(name: str) -> str: diff --git a/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py b/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py index 3c6ed869e..0940ac708 100644 --- a/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py +++ b/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py @@ -160,7 +160,10 @@ def __init__(self) -> None: value_spec=AttributeSpec(default=None, allow_none=True), tags=TagSpec( edi_names=['_atom_site.multiplicity'], - cif_names=['_atom_site.site_symmetry_multiplicity'], + cif_names=[ + '_atom_site.site_symmetry_multiplicity', + '_atom_site_symmetry_multiplicity', + ], ), ) self._occupancy = Parameter( @@ -215,14 +218,22 @@ def __init__(self) -> None: @property def _type_symbol_allowed_values(self) -> list[str]: """ - Chemical symbols accepted by *cryspy*. + Chemical and ionic symbols accepted by *cryspy*. Returns ------- list[str] - Unique element/isotope symbols from the database. - """ - return list({key[1] for key in DATABASE['Isotopes']}) + Unique element/isotope symbols from the database, with + common signed oxidation-state suffixes. + """ + symbols = {key[1] for key in DATABASE['Isotopes']} + ions = { + f'{symbol}{charge}{sign}' + for symbol in symbols + for charge in range(1, 9) + for sign in ('+', '-') + } + return list(symbols | ions) def _resolve_structure_space_group(self) -> object | None: """ diff --git a/src/easydiffraction/io/cif/handler.py b/src/easydiffraction/io/cif/handler.py index 3166720f2..0a2a7316d 100644 --- a/src/easydiffraction/io/cif/handler.py +++ b/src/easydiffraction/io/cif/handler.py @@ -79,13 +79,23 @@ def cif_name(self) -> str: @property def cif_read_names(self) -> list[str]: - """Accepted ``.cif`` import names, in lookup order.""" - return list(dict.fromkeys(self.cif_names)) + """ + Accepted ``.cif`` import names, in lookup order. + + CIF dictionaries use both ``_category.item`` and the older + ``_category_item`` spelling. Gemmi preserves the spelling from + the input document, so add the underscore form of every dotted + name as an import-only alias. Explicitly declared names retain + priority over inferred aliases. + """ + names = list(dict.fromkeys(self.cif_names)) + aliases = [name.replace('.', '_', 1) for name in names if '.' in name] + return list(dict.fromkeys([*names, *aliases])) @property def read_names(self) -> list[str]: """Names accepted on read across both formats (union).""" - return list(dict.fromkeys([self.edi_name, *self._edi_names, *self.cif_names])) + return list(dict.fromkeys([*self.edi_read_names, *self.cif_read_names])) @property def category_name(self) -> str: diff --git a/src/easydiffraction/utils/utils.py b/src/easydiffraction/utils/utils.py index d126af118..80168f709 100644 --- a/src/easydiffraction/utils/utils.py +++ b/src/easydiffraction/utils/utils.py @@ -20,6 +20,7 @@ import numpy as np import pandas as pd import pooch +from filelock import FileLock from packaging.version import Version from rich.markup import escape from uncertainties import UFloat @@ -440,21 +441,28 @@ def _fetch_data_index() -> dict: index_url = _build_data_url('index.json') _validate_url(index_url) - cache_dir = pooch.os_cache('easydiffraction') + cache_dir = pathlib.Path(pooch.os_cache('easydiffraction')) + cache_dir.mkdir(parents=True, exist_ok=True) # Cache under a commit-named file so a ref bump downloads a fresh # index instead of reusing a stale one (data-source-pinning ADR). destination_fname = f'data-index-{_data_index_ref()}.json' + lock_path = cache_dir / f'{destination_fname}.lock' + + # Pooch does not lock ``retrieve`` calls. Parallel processes can + # therefore replace the same cache file while another process opens + # it, which raises PermissionError on Windows. Keep retrieval and + # parsing in one lock. + with FileLock(lock_path): + index_path = pooch.retrieve( + url=index_url, + known_hash=None, + fname=destination_fname, + path=cache_dir, + progressbar=False, + ) - index_path = pooch.retrieve( - url=index_url, - known_hash=None, - fname=destination_fname, - path=cache_dir, - progressbar=False, - ) - - with pathlib.Path(index_path).open('r', encoding='utf-8') as f: - return json.load(f) + with pathlib.Path(index_path).open('r', encoding='utf-8') as f: + return json.load(f) def _existing_project_dir(extraction_dir: pathlib.Path) -> pathlib.Path | None: diff --git a/tests/unit/easydiffraction/analysis/calculators/test_crysfml.py b/tests/unit/easydiffraction/analysis/calculators/test_crysfml.py index b7051730e..7fb0d846c 100644 --- a/tests/unit/easydiffraction/analysis/calculators/test_crysfml.py +++ b/tests/unit/easydiffraction/analysis/calculators/test_crysfml.py @@ -90,6 +90,16 @@ def test_module_import(): assert MUT.__name__ == 'easydiffraction.analysis.calculators.crysfml' +@pytest.mark.parametrize( + ('type_symbol', 'expected'), + [('Fe', 'Fe'), ('57Fe', 'Fe'), ('Fe3+', 'Fe'), ('O2-', 'O')], +) +def test_element_symbol_strips_isotope_and_ionic_notation(type_symbol, expected): + from easydiffraction.analysis.calculators.crysfml import _element_symbol + + assert _element_symbol(type_symbol) == expected + + def test_crysfml_calculate_pattern_applies_absorption(monkeypatch): from easydiffraction.analysis.calculators.crysfml import CrysfmlCalculator from easydiffraction.analysis.corrections import absorption diff --git a/tests/unit/easydiffraction/datablocks/structure/categories/test_atom_sites.py b/tests/unit/easydiffraction/datablocks/structure/categories/test_atom_sites.py index 10f59dabc..3da6ce03c 100644 --- a/tests/unit/easydiffraction/datablocks/structure/categories/test_atom_sites.py +++ b/tests/unit/easydiffraction/datablocks/structure/categories/test_atom_sites.py @@ -77,6 +77,14 @@ def test_type_symbol_setter(self): site.type_symbol = 'Fe' assert site.type_symbol.value == 'Fe' + def test_ionic_type_symbol_setter(self): + from easydiffraction.datablocks.structure.categories.atom_sites.default import AtomSite + + site = AtomSite() + site.type_symbol = 'Fe3+' + + assert site.type_symbol.value == 'Fe3+' + def test_coordinate_setters(self): from easydiffraction.datablocks.structure.categories.atom_sites.default import AtomSite diff --git a/tests/unit/easydiffraction/datablocks/structure/item/test_factory.py b/tests/unit/easydiffraction/datablocks/structure/item/test_factory.py index 148e010ad..0378f1339 100644 --- a/tests/unit/easydiffraction/datablocks/structure/item/test_factory.py +++ b/tests/unit/easydiffraction/datablocks/structure/item/test_factory.py @@ -7,3 +7,36 @@ def test_from_scratch(): m = StructureFactory.from_scratch(name='abc') assert m.name == 'abc' + + +def test_from_cif_str_accepts_underscore_style_structure_tags(): + cif = """\ +data_legacy +_cell_length_a 9.15993(5) +_cell_length_b 9.15993(5) +_cell_length_c 9.15993(5) +_cell_angle_alpha 90 +_cell_angle_beta 90 +_cell_angle_gamma 90 +_symmetry_space_group_name_H-M 'P 21 3' + +loop_ +_atom_site_label +_atom_site_type_symbol +_atom_site_symmetry_multiplicity +_atom_site_fract_x +_atom_site_fract_y +_atom_site_fract_z +_atom_site_B_iso_or_equiv +_atom_site_occupancy +Zr1 Zr4+ 4 0.0003(4) 0.0003(4) 0.0003(4) 0.010(1) 1 +W1 W6+ 4 0.3412(3) 0.3412(3) 0.3412(3) 0.012(1) 1 +""" + + structure = StructureFactory.from_cif_str(cif) + + assert structure.cell.length_a.value == 9.15993 + assert structure.space_group.name_h_m.value == 'P 21 3' + assert structure.atom_sites.names == ['Zr1', 'W1'] + assert structure.atom_sites['Zr1'].type_symbol.value == 'Zr4+' + assert structure.atom_sites['Zr1'].multiplicity.value == 4 diff --git a/tests/unit/easydiffraction/io/cif/test_handler.py b/tests/unit/easydiffraction/io/cif/test_handler.py index 1a7a7e6bc..42b08fb67 100644 --- a/tests/unit/easydiffraction/io/cif/test_handler.py +++ b/tests/unit/easydiffraction/io/cif/test_handler.py @@ -58,7 +58,15 @@ def test_cif_read_names_dedup_and_canonical_first(): handler = TagSpec(edi_names=['_a.x'], cif_names=['_b.y', '_b.z', '_b.y']) assert handler.cif_name == '_b.y' - assert handler.cif_read_names == ['_b.y', '_b.z'] + assert handler.cif_read_names == ['_b.y', '_b.z', '_b_y', '_b_z'] + + +def test_cif_read_names_add_underscore_alias_for_dotted_name(): + from easydiffraction.io.cif.handler import TagSpec + + handler = TagSpec(edi_names=['_cell.length_a']) + + assert handler.cif_read_names == ['_cell.length_a', '_cell_length_a'] def test_cif_names_default_to_edi_names(): @@ -76,4 +84,4 @@ def test_read_names_union_orders_edi_before_cif_and_dedupes(): handler = TagSpec(edi_names=['_a.x'], cif_names=['_a.x', '_b.y']) # Edi name first, then CIF-only aliases, with duplicates removed. - assert handler.read_names == ['_a.x', '_b.y'] + assert handler.read_names == ['_a.x', '_b.y', '_a_x', '_b_y'] diff --git a/tests/unit/easydiffraction/utils/test_utils_coverage.py b/tests/unit/easydiffraction/utils/test_utils_coverage.py index e18671532..596af43a6 100644 --- a/tests/unit/easydiffraction/utils/test_utils_coverage.py +++ b/tests/unit/easydiffraction/utils/test_utils_coverage.py @@ -3,6 +3,8 @@ """Supplementary unit tests for easydiffraction.utils.utils — coverage gaps.""" +import concurrent.futures +import threading import urllib.request import numpy as np @@ -642,6 +644,43 @@ def test_fetch_data_index_reads_cached_json(monkeypatch, tmp_path): assert result == {'1': {'path': 'a.xye'}} +def test_fetch_data_index_serializes_shared_cache_access(monkeypatch, tmp_path): + import json + + import easydiffraction.utils.utils as MUT + + index_file = tmp_path / 'data-index.json' + index_file.write_text(json.dumps({'1': {'path': 'a.xye'}}), encoding='utf-8') + first_retrieve_entered = threading.Event() + release_first_retrieve = threading.Event() + state_lock = threading.Lock() + active_retrieves = 0 + max_active_retrieves = 0 + + def fake_retrieve(url, known_hash, fname, path, progressbar): + nonlocal active_retrieves, max_active_retrieves + with state_lock: + active_retrieves += 1 + max_active_retrieves = max(max_active_retrieves, active_retrieves) + first_retrieve_entered.set() + release_first_retrieve.wait(timeout=1) + with state_lock: + active_retrieves -= 1 + return str(index_file) + + monkeypatch.setattr(MUT.pooch, 'os_cache', lambda name: tmp_path) + monkeypatch.setattr(MUT.pooch, 'retrieve', fake_retrieve) + + with concurrent.futures.ThreadPoolExecutor(max_workers=4) as executor: + futures = [executor.submit(MUT._fetch_data_index) for _ in range(4)] + assert first_retrieve_entered.wait(timeout=1) + release_first_retrieve.set() + results = [future.result(timeout=1) for future in futures] + + assert results == [{'1': {'path': 'a.xye'}}] * 4 + assert max_active_retrieves == 1 + + # --- _existing_project_dir ----------------------------------------------------