Enhances CIF handling in SampleModelFactory

Author: AndrewSazonov

src/easydiffraction/experiments/categories/background/chebyshev.py

@@ -35,7 +35,12 @@ class PolynomialTerm(CategoryItem):
     not break immediately. Tests should migrate to the short names.
     """
 
-    def __init__(self, *, order: int, coef: float) -> None:
+    def __init__(
+        self,
+        *,
+        order=None,
+        coef=None,
+    ) -> None:
         super().__init__()
 
         # Canonical descriptors

src/easydiffraction/experiments/categories/background/line_segment.py

@@ -28,7 +28,12 @@
 class LineSegment(CategoryItem):
     """Single background control point for interpolation."""
 
-    def __init__(self, *, x: float, y: float):
+    def __init__(
+        self,
+        *,
+        x=None,
+        y=None,
+    ):
         super().__init__()
 
         self._x = NumericDescriptor(
@@ -43,7 +48,12 @@ def __init__(self, *, x: float, y: float):
                 default=0.0,
                 content_validator=RangeValidator(),
             ),
-            cif_handler=CifHandler(names=['_pd_background.line_segment_X']),
+            cif_handler=CifHandler(
+                names=[
+                    '_pd_background.line_segment_X',
+                    '_pd_background_line_segment_X',
+                ]
+            ),
         )
         self._y = Parameter(
             name='y',  # TODO: rename to intensity
@@ -57,7 +67,12 @@ def __init__(self, *, x: float, y: float):
                 default=0.0,
                 content_validator=RangeValidator(),
             ),  # TODO: rename to intensity
-            cif_handler=CifHandler(names=['_pd_background.line_segment_intensity']),
+            cif_handler=CifHandler(
+                names=[
+                    '_pd_background.line_segment_intensity',
+                    '_pd_background_line_segment_intensity',
+                ]
+            ),
         )
 
         self._identity.category_code = 'background'

src/easydiffraction/experiments/categories/excluded_regions.py

@@ -21,8 +21,8 @@ class ExcludedRegion(CategoryItem):
     def __init__(
         self,
         *,
-        start: float,
-        end: float,
+        start=None,
+        end=None,
     ):
         super().__init__()
 

src/easydiffraction/experiments/categories/linked_phases.py

@@ -19,8 +19,8 @@ class LinkedPhase(CategoryItem):
     def __init__(
         self,
         *,
-        id: str,  # TODO: need new name instead of id
-        scale: float,
+        id=None,  # TODO: need new name instead of id
+        scale=None,
     ):
         super().__init__()
 

src/easydiffraction/experiments/experiment/factory.py

@@ -13,6 +13,10 @@
 from easydiffraction.experiments.experiment.enums import RadiationProbeEnum
 from easydiffraction.experiments.experiment.enums import SampleFormEnum
 from easydiffraction.experiments.experiment.enums import ScatteringTypeEnum
+from easydiffraction.io.cif.parse import document_from_path
+from easydiffraction.io.cif.parse import name_from_block
+from easydiffraction.io.cif.parse import pick_sole_block
+from easydiffraction.io.cif.serialize import datastore_from_cif
 
 
 class ExperimentFactory(FactoryBase):
@@ -42,76 +46,25 @@ class ExperimentFactory(FactoryBase):
     }
 
     @classmethod
-    def create(cls, **kwargs):
-        """Create an `ExperimentBase` using a validated argument
-        combination.
+    def _make_experiment_type(cls, kwargs):
+        """Helper to construct an ExperimentType from keyword arguments,
+        using defaults as needed.
         """
-        # Check for valid argument combinations
-        user_args = {k for k, v in kwargs.items() if v is not None}
-        cls._validate_args(user_args, cls._ALLOWED_ARG_SPECS, cls.__name__)
-
-        # Validate enum arguments if provided
-        if 'sample_form' in kwargs:
-            SampleFormEnum(kwargs['sample_form'])
-        if 'beam_mode' in kwargs:
-            BeamModeEnum(kwargs['beam_mode'])
-        if 'radiation_probe' in kwargs:
-            RadiationProbeEnum(kwargs['radiation_probe'])
-        if 'scattering_type' in kwargs:
-            ScatteringTypeEnum(kwargs['scattering_type'])
-
-        # Dispatch to the appropriate creation method
-        if 'cif_path' in kwargs:
-            return cls._create_from_cif_path(kwargs['cif_path'])
-        elif 'cif_str' in kwargs:
-            return cls._create_from_cif_str(kwargs['cif_str'])
-        elif 'data_path' in kwargs:
-            return cls._create_from_data_path(kwargs)
-        elif 'name' in kwargs:
-            return cls._create_without_data(kwargs)
-
-    # -------------
-    # gemmi helpers
-    # -------------
-
-    # TODO: Move to a common CIF utility module? io.cif.parse?
-    @staticmethod
-    def _read_cif_document_from_path(path: str) -> gemmi.cif.Document:
-        """Read a CIF document from a file path."""
-        return gemmi.cif.read_file(path)
-
-    # TODO: Move to a common CIF utility module? io.cif.parse?
-    @staticmethod
-    def _read_cif_document_from_string(text: str) -> gemmi.cif.Document:
-        """Read a CIF document from a raw text string."""
-        return gemmi.cif.read_string(text)
-
-    # TODO: Move to a common CIF utility module? io.cif.parse?
-    @classmethod
-    def _pick_first_block(
-        cls,
-        doc: gemmi.cif.Document,
-    ) -> gemmi.cif.Block:
-        """Pick the first experimental block from a CIF document."""
-        try:
-            return doc.sole_block()
-        except Exception:
-            return doc[0]
-
-    # TODO: Move to a common CIF utility module? io.cif.parse?
-    @classmethod
-    def _extract_name_from_block(cls, block: gemmi.cif.Block) -> str:
-        """Extract a model name from the CIF block name."""
-        return block.name or 'model'
+        return ExperimentType(
+            sample_form=kwargs.get('sample_form', SampleFormEnum.default()),
+            beam_mode=kwargs.get('beam_mode', BeamModeEnum.default()),
+            radiation_probe=kwargs.get('radiation_probe', RadiationProbeEnum.default()),
+            scattering_type=kwargs.get('scattering_type', ScatteringTypeEnum.default()),
+        )
 
     # TODO: Move to a common CIF utility module? io.cif.parse?
     @classmethod
-    def _create_experiment_from_block(
+    def _create_from_gemmi_block(
         cls,
         block: gemmi.cif.Block,
     ) -> ExperimentBase:
         """Build a model instance from a single CIF block."""
-        name = cls._extract_name_from_block(block)
+        name = name_from_block(block)
 
         # TODO: experiment type need to be read from CIF block
         kwargs = {'beam_mode': BeamModeEnum.CONSTANT_WAVELENGTH}
@@ -123,22 +76,24 @@ def _create_experiment_from_block(
         expt_class = cls._SUPPORTED[scattering_type][sample_form]
         expt_obj = expt_class(name=name, type=expt_type)
 
-        # TODO: Read all categories from CIF block into experiment
+        # Read all categories from CIF block
+        for category in expt_obj.categories:
+            category.from_cif(block)
 
-        # TODO: Read data from CIF block into experiment datastore
+        # Populate experiment datastore from CIF block
+        datastore_from_cif(expt_obj, block)
 
         return expt_obj
 
-    # -------------------------------
-    # Private creation helper methods
-    # -------------------------------
-
     @classmethod
-    def _create_from_cif_path(cls, cif_path):
+    def _create_from_cif_path(
+        cls,
+        cif_path: str,
+    ) -> ExperimentBase:
         """Create an experiment from a CIF file path."""
-        doc = cls._read_cif_document_from_path(cif_path)
-        block = cls._pick_first_block(doc)
-        return cls._create_experiment_from_block(block)
+        doc = document_from_path(cif_path)
+        block = pick_sole_block(doc)
+        return cls._create_from_gemmi_block(block)
 
     @staticmethod
     def _create_from_cif_str(cif_str):
@@ -182,29 +137,35 @@ def _create_without_data(cls, kwargs):
         return expt_obj
 
     @classmethod
-    def _make_experiment_type(cls, kwargs):
-        """Helper to construct an ExperimentType from keyword arguments,
-        using defaults as needed.
+    def create(cls, **kwargs):
+        """Create an `ExperimentBase` using a validated argument
+        combination.
         """
-        return ExperimentType(
-            sample_form=kwargs.get('sample_form', SampleFormEnum.default()),
-            beam_mode=kwargs.get('beam_mode', BeamModeEnum.default()),
-            radiation_probe=kwargs.get('radiation_probe', RadiationProbeEnum.default()),
-            scattering_type=kwargs.get('scattering_type', ScatteringTypeEnum.default()),
+        # TODO: move to FactoryBase
+        # Check for valid argument combinations
+        user_args = {k for k, v in kwargs.items() if v is not None}
+        cls._validate_args(
+            present=user_args,
+            allowed_specs=cls._ALLOWED_ARG_SPECS,
+            factory_name=cls.__name__,  # TODO: move to FactoryBase
         )
 
-    @staticmethod
-    def _is_valid_args(user_args):
-        """Validate user argument set against allowed combinations.
+        # Validate enum arguments if provided
+        if 'sample_form' in kwargs:
+            SampleFormEnum(kwargs['sample_form'])
+        if 'beam_mode' in kwargs:
+            BeamModeEnum(kwargs['beam_mode'])
+        if 'radiation_probe' in kwargs:
+            RadiationProbeEnum(kwargs['radiation_probe'])
+        if 'scattering_type' in kwargs:
+            ScatteringTypeEnum(kwargs['scattering_type'])
 
-        Returns True if the argument set matches any valid combination,
-        else False.
-        """
-        user_arg_set = set(user_args)
-        for arg_set in ExperimentFactory._valid_arg_sets:
-            required = set(arg_set['required'])
-            optional = set(arg_set['optional'])
-            # Must have all required, and only required+optional
-            if required.issubset(user_arg_set) and user_arg_set <= (required | optional):
-                return True
-        return False
+        # Dispatch to the appropriate creation method
+        if 'cif_path' in kwargs:
+            return cls._create_from_cif_path(kwargs['cif_path'])
+        elif 'cif_str' in kwargs:
+            return cls._create_from_cif_str(kwargs['cif_str'])
+        elif 'data_path' in kwargs:
+            return cls._create_from_data_path(kwargs)
+        elif 'name' in kwargs:
+            return cls._create_without_data(kwargs)

src/easydiffraction/io/cif/parse.py

@@ -0,0 +1,25 @@
+# SPDX-FileCopyrightText: 2021-2025 EasyDiffraction contributors <https://github.com/easyscience/diffraction>
+# SPDX-License-Identifier: BSD-3-Clause
+
+import gemmi
+
+
+def document_from_path(path: str) -> gemmi.cif.Document:
+    """Read a CIF document from a file path."""
+    return gemmi.cif.read_file(path)
+
+
+def document_from_string(text: str) -> gemmi.cif.Document:
+    """Read a CIF document from a raw text string."""
+    return gemmi.cif.read_string(text)
+
+
+def pick_sole_block(doc: gemmi.cif.Document) -> gemmi.cif.Block:
+    """Pick the sole data block from a CIF document."""
+    return doc.sole_block()
+
+
+def name_from_block(block: gemmi.cif.Block) -> str:
+    """Extract a model name from the CIF block name."""
+    # TODO: Need validator or normalization?
+    return block.name