Merge pull request #309 from bluesky/307-add_reflections-crashes

avoid crash in add_reflections, add axes_(r,w,c)

RELEASE_NOTES.rst

@@ -30,6 +30,7 @@ New Features and/or Enhancements
 * Add ``SoleilNanoscopiumRobot`` diffractometer geometry.
 * Add ``SoleilSixsMed2p3v2`` diffractometer geometry.
 * Add ``user.current_diffractometer()`` function.
+* Add ``axes_r``, ``axes_w``, & ``axes_c`` properties to both ``calc`` & ``engine``.
 * Export and reload diffractometer configuration as JSON string, YAML string, or Python dictionary.
 
 Fixes
@@ -44,11 +45,12 @@ Maintenance
 * Add ``apischema`` to package requirements.
 * Add test for ``or_swap()``.
 * Change documentation theme to pydata-sphinx-theme.
+* Documentation ZIP file uploaded as artifact with each build.  Great for review!
 * Expand testing to to Py3.8 - Py3.11.
 * Fix code in ``util.restore_reflections()`` that failed unit tests locally.
 * Make it easier to find the SPEC command cross-reference table.
 * Update packaging to latest PyPA recommendations.
-* Documentation ZIP file uploaded as artifact with each build.  Great for review!
+* Validate user input to sample.add_reflection().
 
 v1.0.4 (released 2023-10-06)
 ======================================

hkl/calc.py

@@ -281,6 +281,29 @@ def engine(self, engine):
 
         self._re_init()
 
+    def _canonical2user(self, canonical):
+        """Convert canonical axis names to user (renames)."""
+        if len(self._axis_name_to_renamed) > 0:
+            axis = self._axis_name_to_renamed[canonical]
+        else:
+            axis = canonical
+        return axis
+
+    @property
+    def axes_r(self):
+        """User-defined real-space axes used for forward() calculation."""
+        return [self._canonical2user(ax) for ax in self._engine.axes_r]
+
+    @property
+    def axes_w(self):
+        """User-defined real-space axes written by forward() calculation."""
+        return [self._canonical2user(ax) for ax in self._engine.axes_w]
+
+    @property
+    def axes_c(self):
+        """User-defined real-space axes held constant during forward() calculation."""
+        return [self._canonical2user(ax) for ax in self._engine.axes_c]
+
     @property
     def geometry_name(self):
         """Name of this geometry, as defined in **libhkl**."""

hkl/engine.py

@@ -26,6 +26,10 @@
 logger = logging.getLogger(__name__)
 
 
+AXES_READ = 0
+AXES_WRITTEN = 1
+
+
 class Parameter(object):
     """HKL library parameter object
 
@@ -236,6 +240,21 @@ def name(self):
         """Name of this engine."""
         return self._engine.name_get()
 
+    @property
+    def axes_c(self):
+        """HKL real axis names (held constant during forward() computation)."""
+        return [axis for axis in self.axes_r if axis not in self.axes_w]
+
+    @property
+    def axes_r(self):
+        """HKL real axis names (read-only)."""
+        return self._engine.axis_names_get(AXES_READ)
+
+    @property
+    def axes_w(self):
+        """HKL real axis names (written by forward() computation)."""
+        return self._engine.axis_names_get(AXES_WRITTEN)
+
     @property
     def mode(self):
         """HKL calculation mode (see also `HklCalc.modes`)"""

hkl/sample.py

@@ -333,20 +333,20 @@ def reflections(self, refls):
         for refl in refls:
             self.add_reflection(*refl)
 
-    def add_reflection(self, h, k, l, position=None, detector=None, compute_ub=False):
+    def add_reflection(self, h: float, k: float, l: float, position=None, detector=None, compute_ub: bool = False):
         """Add a reflection, optionally specifying the detector to use
 
         Parameters
         ----------
-        h : float
+        h : (int, float)
             Reflection h
-        k : float
+        k : (int, float)
             Reflection k
-        l : float
+        l : (int, float)
             Reflection l
         detector : Hkl.Detector, optional
             The detector
-        position : tuple or namedtuple, optional
+        position : list, tuple, or namedtuple, optional
             The physical motor position that this reflection corresponds to
             If not specified, the current geometry of the calculation engine is
             assumed.
@@ -364,7 +364,46 @@ def add_reflection(self, h, k, l, position=None, detector=None, compute_ub=False
             r1 = self._sample.reflections_get()[-1]
 
         with TemporaryGeometry(calc):
-            if position is not None:
+
+            def has_valid_position(pos):
+                """Raise if invalid, otherwise return boolean."""
+                if pos is None:
+                    # so use the current motor positions
+                    return False
+                elif type(pos).__name__.startswith("Pos"):
+                    # This is (probably) a calc.Position namedtuple
+                    if False in [isinstance(v, (int, float)) for v in pos]:
+                        raise TypeError(f"All values must be numeric, received {pos!r}")
+                    if pos._fields != tuple(calc.physical_axis_names):
+                        # fmt: off
+                        raise KeyError(
+                            f"Wrong axes names.  Expected {calc.physical_axis_names},"
+                            f" received {pos._fields}"
+                        )
+                        # fmt: on
+                    return True
+                elif type(pos).__name__ in "list tuple".split():
+                    # note: isinstance(pos, (list, tuple)) includes namedtuple
+                    if len(pos) != len(calc.physical_axis_names):
+                        # fmt: off
+                        raise ValueError(
+                            f"Expected {len(calc.physical_axis_names)}"
+                            f" positions, received {pos!r}"
+                        )
+                        # fmt: on
+                    if False in [isinstance(v, (int, float)) for v in pos]:
+                        raise TypeError(f"All values must be numeric, received {pos!r}")
+                    return True
+                elif isinstance(pos, (int, float)):
+                    raise TypeError(f"Expected positions, received {pos!r}")
+                # fmt: off
+                raise TypeError(
+                    f"Expected list, tuple, or calc.Position() object,"
+                    f" received {pos!r}"
+                )
+                # fmt: on
+
+            if has_valid_position(position):
                 calc.physical_positions = position
             r2 = self._sample.add_reflection(calc._geometry, detector, h, k, l)
 

hkl/tests/conftest.py

@@ -12,6 +12,7 @@
 from .. import SimulatedE4CV
 from .. import SimulatedE6C
 from .. import SimulatedK4CV
+from .. import SimulatedK6C
 
 
 @pytest.fixture
@@ -65,6 +66,15 @@ def k4cv():
     return diffractometer
 
 
+@pytest.fixture
+def k6c():
+    """Standard K6C."""
+    diffractometer = SimulatedK6C("", name="k6c")
+    diffractometer.wait_for_connection()
+    diffractometer._update_calc_energy()
+    return diffractometer
+
+
 @pytest.fixture
 def tardis():
     class Tardis(SimMixin, E6C):