diff --git a/RELEASE_NOTES.rst b/RELEASE_NOTES.rst index 7e40d519..092c71b0 100644 --- a/RELEASE_NOTES.rst +++ b/RELEASE_NOTES.rst @@ -31,6 +31,8 @@ New Features and/or Enhancements * Add ``SoleilSixsMed2p3v2`` diffractometer geometry. * Export and restore diffractometer configuration as JSON string, YAML string, Python dictionary, or file. * 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 +46,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(). Deprecations ------------ diff --git a/hkl/calc.py b/hkl/calc.py index cce3deae..a2c4dde6 100644 --- a/hkl/calc.py +++ b/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**.""" diff --git a/hkl/diffract.py b/hkl/diffract.py index 405f6256..390f9cfb 100644 --- a/hkl/diffract.py +++ b/hkl/diffract.py @@ -47,6 +47,7 @@ class Diffractometer(PseudoPositioner): ~inverse ~forward_solutions_table ~apply_constraints + ~get_axis_constraints ~reset_constraints ~show_constraints ~undo_last_constraints @@ -540,6 +541,10 @@ def _constraints_for_databroker(self): ] # fmt: on + def get_axis_constraints(self, axis): + """Show the constraints for one axis.""" + return self._constraints_dict[axis] + def show_constraints(self, fmt="simple", printing=True): """Print the current constraints in a table.""" tbl = pyRestTable.Table() diff --git a/hkl/engine.py b/hkl/engine.py index 109e820e..ed34fac9 100644 --- a/hkl/engine.py +++ b/hkl/engine.py @@ -26,6 +26,10 @@ logger = logging.getLogger(__name__) +AXES_READ = 0 +AXES_WRITTEN = 1 + + class Parameter(object): """HKL library parameter object @@ -239,6 +243,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`)""" diff --git a/hkl/sample.py b/hkl/sample.py index e67a0f69..19e4c00b 100644 --- a/hkl/sample.py +++ b/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) diff --git a/hkl/tests/conftest.py b/hkl/tests/conftest.py index cc339a65..a851aec2 100644 --- a/hkl/tests/conftest.py +++ b/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): diff --git a/hkl/tests/test_diffract.py b/hkl/tests/test_diffract.py index a410e54b..0342d6c4 100644 --- a/hkl/tests/test_diffract.py +++ b/hkl/tests/test_diffract.py @@ -347,3 +347,13 @@ class Q4C(hkl.E4CV): assert value.omega == 0.0 assert value.chi == 0.0 assert value.phi == 0.0 + + +@pytest.mark.parametrize("axis", "alpha phi omega".split()) +def test_get_constraint(axis, fourc): + if axis in fourc.calc._geometry.axis_names_get(): + constraint = fourc.get_axis_constraints(axis) + assert isinstance(constraint, Constraint) + else: + with pytest.raises(KeyError): + fourc.get_axis_constraints(axis) diff --git a/hkl/tests/test_i307.py b/hkl/tests/test_i307.py new file mode 100644 index 00000000..24158a42 --- /dev/null +++ b/hkl/tests/test_i307.py @@ -0,0 +1,238 @@ +""" +Tests for changes due to issues #307 & #308. +""" + +from collections import namedtuple +from contextlib import nullcontext as does_not_raise + +import pytest + +from ..util import get_position_tuple + + +@pytest.mark.parametrize( + "miller, context, message", + [ + [(1, 2, 3), does_not_raise(), None], + [(1.0, 2.0, 3.0), does_not_raise(), None], + [("1", 2, 3), pytest.raises(TypeError), "Must be number, not str"], + [(1, 2, "3"), pytest.raises(TypeError), "Must be number, not str"], + [([1], 2, 3), pytest.raises(TypeError), "Must be number, not list"], + [(object, 2, 3), pytest.raises(TypeError), "Must be number, not type"], + [(None, 2, 3), pytest.raises(TypeError), "does not allow None as a value"], + [None, pytest.raises(TypeError), "argument after * must be an iterable, not NoneType"], + [((1,), 2, 3), pytest.raises(TypeError), "Must be number, not tuple"], + [ + # Tests that h, k, l was omitted, only a position was supplied. + # This is one of the problems reported. + namedtuple("PosAnything", "a b c d".split())(1, 2, 3, 4), + pytest.raises(TypeError), + "Expected positions, received 4", + ], + ], +) +def test_miller_args(miller, context, message, e4cv): + """Test the Miller indices arguments: h, k, l.""" + sample = e4cv.calc.sample + assert sample is not None + + with context as info: + sample.add_reflection(*miller) + if message is not None: + assert message in str(info.value) + + +# fmt: off +@pytest.mark.parametrize( + "miller, angles, context, message", + [ + # None + [(-1, -2, -3), None, does_not_raise(), None], + # tuple + [(-1, -2, -3), (1, 2, 3, 4), does_not_raise(), None], + # list + [(-1, -2, -3), [1, 2, 3, 4], does_not_raise(), None], + # dict + [ + (-1, -2, -3), + {"omega": 1, "chi": 2, "phi": 3, "tth": 4}, + pytest.raises(TypeError), + "Expected list, tuple, or calc.Position() object,", + ], + # namedtuple + [(-1, -2, -3), "namedtuple 1 2 3 4", does_not_raise(), None], + # calc.Position object + [(-1, -2, -3), "Position 1 2 3 4", does_not_raise(), None], + # Unacceptable namedtuple (does not start with "Pos") + [ + (-10, -2, -3), + namedtuple("Not_Position", "a b c d".split())(1, 2, 3, 4), + pytest.raises(TypeError), + "Expected list, tuple, or calc.Position() object", + ], + # Acceptable namedtuple (starts with "Pos") type + # yet the axis names are wrong. + [ + (-10, -2, -3), + namedtuple("Positron", "a b c d".split())(1, 2, 3, 4), + pytest.raises(KeyError), + "Wrong axes names. Expected [", + ], + # Acceptable namedtuple (starts with "Pos") type + # yet values must all be numeric. + [ + (-10, -2, -3), + namedtuple("Post", "omega chi phi tth".split())(1, "2", 3, 4), + pytest.raises(TypeError), + "All values must be numeric", + ], + # Acceptable namedtuple (starts with "Pos") type & content. + [ + (-10, -2, -3), + namedtuple("Posh", "omega chi phi tth".split())(1, 2, 3, 4), + does_not_raise(), + None, + ], + ] +) +# fmt: on +def test_position_args(miller, angles, context, message, e4cv): + """Test sample.add_reflection with the different ways positions can be entered.""" + assert len(miller) == 3 + + calc = e4cv.calc + assert calc is not None + + sample = calc.sample + assert sample is not None + + if isinstance(angles, str): + # These constructs require some additional setup. + if angles.startswith("Position"): + angles = calc.Position(*map(float, angles.split()[1:])) + elif angles.startswith("namedtuple"): + # fmt: off + angles = get_position_tuple("omega chi phi tth".split())( + *list(map(float, angles.split()[1:])) + ) + # fmt: on + + with context as info: + sample.add_reflection(*miller, angles) + if message is not None: + assert message in str(info.value) + + +@pytest.mark.parametrize( + "args, context, message", + [ + # wrong number of reals, tuple + [[1, 2, 3, (4, 5)], pytest.raises(ValueError), "Expected 4 positions,"], + # wrong number of reals, list + [[1, 2, 3, [4, 5]], pytest.raises(ValueError), "Expected 4 positions,"], + # wrong number of reals, namedtuple + [ + [1, 2, 3, namedtuple("Position", "omega chi phi".split())(1, 2, 3)], + pytest.raises(KeyError), + "Wrong axes names. Expected [", + ], + # wrong representation of position + [[1, 2, 3, 4], pytest.raises(TypeError), "Expected positions"], + # pseudos provided as tuple + [[(1, 2, 3), (4, 5, 6, 7)], pytest.raises(TypeError), "missing 1 required"], + # only pseudos (uses current positions so no problem) + [[1, 2, 3], does_not_raise(), None], + [[1, 2, 3, namedtuple("Position", "omega chi phi tth".split())(4, 5, 6, 7)], does_not_raise(), None], + ], +) +def test_add_reflection(args, context, message, e4cv): + calc = e4cv.calc + sample = calc.sample + assert sample is not None + + with context as info: + sample.add_reflection(*args) + if message is not None: + assert message in str(info.value) + + +@pytest.mark.parametrize( + "dname, mode, n_r, n_w, e_a_c, c_a_c", + [ + # all modes on each common geometry, some with canonical names, some with renames + # If not renamed, set c_a_c to `None` + # fmt: off + ["e4cv", "bissector", 4, 4, "", None], + ["e4cv", "constant_chi", 4, 3, "chi", None], + ["e4cv", "constant_omega", 4, 3, "omega", None], + ["e4cv", "constant_phi", 4, 3, "phi", None], + + ["e4cv_renamed", "bissector", 4, 4, "", ""], + ["e4cv_renamed", "constant_chi", 4, 3, "chi", "chi"], + ["e4cv_renamed", "constant_omega", 4, 3, "omega", "theta"], + ["e4cv_renamed", "constant_phi", 4, 3, "phi", "phi"], + ["e4cv_renamed", "double_diffraction", 4, 4, "", ""], + + ["e6c", "bissector_horizontal", 6, 5, "delta", None], + ["e6c", "bissector_vertical", 6, 4, "mu gamma", None], + ["e6c", "constant_phi_vertical", 6, 3, "mu phi gamma", None], + ["e6c", "psi_constant_horizontal", 6, 4, "mu delta", None], + ["e6c", "psi_constant_vertical", 6, 4, "mu gamma", None], + + ["tardis", "bissector_horizontal", 6, 5, "delta", "gamma"], + ["tardis", "bissector_vertical", 6, 4, "mu gamma", "theta delta"], + ["tardis", "constant_phi_vertical", 6, 3, "mu phi gamma", "theta phi delta"], + ["tardis", "psi_constant_horizontal", 6, 4, "mu delta", "theta gamma"], + ["tardis", "psi_constant_vertical", 6, 4, "mu gamma", "theta delta"], + ["tardis", "constant_chi_vertical", 6, 3, "mu chi gamma", "theta chi delta"], + ["tardis", "constant_mu_horizontal", 6, 3, "mu omega delta", "theta omega gamma"], + ["tardis", "constant_omega_vertical", 6, 3, "mu omega gamma", "theta omega delta"], + ["tardis", "double_diffraction_horizontal", 6, 4, "omega delta", "omega gamma"], + ["tardis", "double_diffraction_vertical", 6, 4, "mu gamma", "theta delta"], + ["tardis", "lifting_detector_mu", 6, 3, "omega chi phi", "omega chi phi"], + ["tardis", "lifting_detector_omega", 6, 3, "mu chi phi", "theta chi phi"], + ["tardis", "lifting_detector_phi", 6, 3, "mu omega chi", "theta omega chi"], + + ["k4cv", "bissector", 4, 4, "", None], + ["k4cv", "constant_chi", 4, 4, "", None], + ["k4cv", "constant_omega", 4, 4, "", None], + ["k4cv", "constant_phi", 4, 4, "", None], + ["k4cv", "psi_constant", 4, 4, "", None], + ["k4cv", "double_diffraction", 4, 4, "", None], + + ["k6c", "bissector_horizontal", 6, 5, "delta", None], + ["k6c", "bissector_vertical", 6, 4, "mu gamma", None], + ["k6c", "constant_incidence", 6, 5, "mu", None], + ["k6c", "constant_kphi_horizontal", 6, 4, "kphi delta", None], + ["k6c", "constant_omega_vertical", 6, 4, "mu gamma", None], + ["k6c", "constant_phi_horizontal", 6, 5, "delta", None], + ["k6c", "constant_phi_vertical", 6, 4, "mu gamma", None], + ["k6c", "double_diffraction_horizontal", 6, 5, "delta", None], + ["k6c", "double_diffraction_vertical", 6, 4, "mu gamma", None], + ["k6c", "lifting_detector_komega", 6, 3, "mu kappa kphi", None], + ["k6c", "lifting_detector_kphi", 6, 3, "mu komega kappa", None], + ["k6c", "lifting_detector_mu", 6, 3, "komega kappa kphi", None], + ["k6c", "psi_constant_vertical", 6, 4, "mu gamma", None], + # fmt: on + ], +) +def test_engine_axes(dname, mode, n_r, n_w, e_a_c, c_a_c, e4cv, e4cv_renamed, e6c, k4cv, k6c, tardis): + diffractometer_choices = dict(e4cv=e4cv, e4cv_renamed=e4cv_renamed, e6c=e6c, k4cv=k4cv, k6c=k6c, tardis=tardis) + assert dname in diffractometer_choices + + dfrctmtr = diffractometer_choices[dname] + calc = dfrctmtr.calc + engine = calc.engine + + assert mode in engine.modes + engine.mode = mode # tests below apply to this engine and mode + + # engine: canonical names + assert len(engine.axes_r) == n_r + assert len(engine.axes_w) == n_w + assert engine.axes_c == e_a_c.split() # e_a_c : engine axes_c + + # calc: user names (might be renamed from canonical) + assert len(calc.axes_r) == n_r + assert len(calc.axes_w) == n_w + assert calc.axes_c == (c_a_c or e_a_c).split() # c_a_c : calc axes_c