Merge pull request diffpy#209 from bobleesj/scattering-obj-valid

Check valid x and y array lenghts and xtypes in `insert_scattering_quantity`

Author: sbillinge

news/scattering-obj-valid.rst

@@ -0,0 +1,23 @@
+**Added:**
+
+* validate xtype belongs to XQUANTITIES during DiffractionObject init
+
+**Changed:**
+
+* <news item>
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>

src/diffpy/utils/diffraction_objects.py

@@ -21,8 +21,15 @@
 
 def _xtype_wmsg(xtype):
     return (
-        f"I don't know how to handle the xtype, '{xtype}'. Please rerun specifying an "
-        f"xtype from {*XQUANTITIES, }"
+        f"I don't know how to handle the xtype, '{xtype}'. "
+        f"Please rerun specifying an xtype from {*XQUANTITIES, }"
+    )
+
+
+def _setter_wmsg(attribute):
+    return (
+        f"Direct modification of attribute '{attribute}' is not allowed. "
+        f"Please use 'insert_scattering_quantity' to modify '{attribute}'.",
     )
 
 
@@ -45,6 +52,7 @@ def __init__(
             xarray = np.empty(0)
         if yarray is None:
             yarray = np.empty(0)
+
         self.insert_scattering_quantity(xarray, yarray, xtype)
 
     def __eq__(self, other):
@@ -186,11 +194,16 @@ def all_arrays(self):
         return self._all_arrays
 
     @all_arrays.setter
-    def all_arrays(self, value):
-        raise AttributeError(
-            "Direct modification of attribute 'all_arrays' is not allowed."
-            "Please use 'insert_scattering_quantity' to modify `all_arrays`."
-        )
+    def all_arrays(self, _):
+        raise AttributeError(_setter_wmsg("all_arrays"))
+
+    @property
+    def input_xtype(self):
+        return self._input_xtype
+
+    @input_xtype.setter
+    def input_xtype(self, _):
+        raise AttributeError(_setter_wmsg("input_xtype"))
 
     def set_angles_from_list(self, angles_list):
         self.angles = angles_list
@@ -261,7 +274,7 @@ def _set_array_from_range(self, begin, end, step_size=None, n_steps=None):
 
     def get_array_index(self, value, xtype=None):
         """
-        returns the index of the closest value in the array associated with the specified xtype
+        Return the index of the closest value in the array associated with the specified xtype.
 
         Parameters
         ----------
@@ -276,7 +289,7 @@ def get_array_index(self, value, xtype=None):
         """
 
         if xtype is None:
-            xtype = self.input_xtype
+            xtype = self._input_xtype
         array = self.on_xtype(xtype)[0]
         if len(array) == 0:
             raise ValueError(f"The '{xtype}' array is empty. Please ensure it is initialized.")
@@ -333,9 +346,18 @@ def insert_scattering_quantity(
         Nothing.  Updates the object in place.
 
         """
+
+        # Check xarray and yarray have the same length
+        if len(xarray) != len(yarray):
+            raise ValueError(
+                "'xarray' and 'yarray' must have the same length. "
+                "Please re-initialize 'DiffractionObject' or re-run the method 'insert_scattering_quantity' "
+                "with 'xarray' and 'yarray' of identical length."
+            )
+
         self._set_xarrays(xarray, xtype)
         self._all_arrays[:, 0] = yarray
-        self.input_xtype = xtype
+        self._input_xtype = xtype
         # only update these optional values if non-empty quantities are passed to avoid overwriting
         # valid data inadvertently
         if metadata:
@@ -347,12 +369,17 @@ def insert_scattering_quantity(
         if wavelength is not None:
             self.wavelength = wavelength
 
+        # Check xtype is valid. An empty string is the default value.
+        if xtype != "":
+            if xtype not in XQUANTITIES:
+                raise ValueError(_xtype_wmsg(xtype))
+
     def _get_original_array(self):
-        if self.input_xtype in QQUANTITIES:
+        if self._input_xtype in QQUANTITIES:
             return self.on_q(), "q"
-        elif self.input_xtype in ANGLEQUANTITIES:
+        elif self._input_xtype in ANGLEQUANTITIES:
             return self.on_tth(), "tth"
-        elif self.input_xtype in DQUANTITIES:
+        elif self._input_xtype in DQUANTITIES:
             return self.on_d(), "d"
 
     def on_q(self):
@@ -365,8 +392,8 @@ def on_d(self):
         return [self.all_arrays[:, 3], self.all_arrays[:, 0]]
 
     def scale_to(self, target_diff_object, xtype=None, xvalue=None):
-        f"""
-        returns a new diffraction object which is the current object but recaled in y to the target
+        """
+        Return a new diffraction object which is the current object but recaled in y to the target
 
         Parameters
         ----------
@@ -401,8 +428,8 @@ def scale_to(self, target_diff_object, xtype=None, xvalue=None):
         return scaled
 
     def on_xtype(self, xtype):
-        f"""
-        return a list of two 1D np array with x and y data, raise an error if the specified xtype is invalid
+        """
+        Return a list of two 1D np array with x and y data, raise an error if the specified xtype is invalid
 
         Parameters
         ----------

tests/test_diffraction_objects.py

@@ -211,16 +211,15 @@ def test_on_xtype():
     assert np.allclose(test.on_xtype("d"), [np.array([12.13818, 6.28319]), np.array([1, 2])])
 
 
-def test_on_xtype_bad():
-    test = DiffractionObject()
+def test_init_invalid_xtype():
     with pytest.raises(
         ValueError,
         match=re.escape(
-            f"I don't know how to handle the xtype, 'invalid'. Please rerun specifying an "
-            f"xtype from {*XQUANTITIES, }"
+            f"I don't know how to handle the xtype, 'invalid_type'. "
+            f"Please rerun specifying an xtype from {*XQUANTITIES, }"
         ),
     ):
-        test.on_xtype("invalid")
+        DiffractionObject(xtype="invalid_type")
 
 
 params_index = [
@@ -287,7 +286,7 @@ def test_dump(tmp_path, mocker):
         {
             "_all_arrays": np.empty(shape=(0, 4)),  # instantiate empty
             "metadata": {},
-            "input_xtype": "",
+            "_input_xtype": "",
             "name": "",
             "scat_quantity": None,
             "qmin": np.float64(np.inf),
@@ -304,7 +303,7 @@ def test_dump(tmp_path, mocker):
         {
             "_all_arrays": np.empty(shape=(0, 4)),
             "metadata": {"thing": "1", "another": "2"},
-            "input_xtype": "",
+            "_input_xtype": "",
             "name": "test",
             "scat_quantity": "x-ray",
             "qmin": np.float64(np.inf),
@@ -332,7 +331,7 @@ def test_dump(tmp_path, mocker):
                 ]
             ),
             "metadata": {},
-            "input_xtype": "tth",
+            "_input_xtype": "tth",
             "name": "",
             "scat_quantity": None,
             "qmin": np.float64(0.0),
@@ -361,7 +360,7 @@ def test_dump(tmp_path, mocker):
                 ]
             ),
             "metadata": {},
-            "input_xtype": "d",
+            "_input_xtype": "d",
             "name": "",
             "scat_quantity": "x-ray",
             "qmin": np.float64(0.0),
@@ -406,12 +405,39 @@ def test_all_array_setter():
     # Attempt to directly modify the property
     with pytest.raises(
         AttributeError,
-        match="Direct modification of attribute 'all_arrays' is not allowed."
-        "Please use 'insert_scattering_quantity' to modify `all_arrays`.",
+        match="Direct modification of attribute 'all_arrays' is not allowed. "
+        "Please use 'insert_scattering_quantity' to modify 'all_arrays'.",
     ):
         actual_do.all_arrays = np.empty((4, 4))
 
 
+def test_xarray_yarray_length_mismatch():
+    with pytest.raises(
+        ValueError,
+        match="'xarray' and 'yarray' must have the same length. "
+        "Please re-initialize 'DiffractionObject' or re-run the method 'insert_scattering_quantity' "
+        "with 'xarray' and 'yarray' of identical length",
+    ):
+        DiffractionObject(xarray=np.array([1.0, 2.0]), yarray=np.array([0.0, 0.0, 0.0]))
+
+
+def test_input_xtype_getter():
+    do = DiffractionObject(xtype="tth")
+    assert do.input_xtype == "tth"
+
+
+def test_input_xtype_setter():
+    do = DiffractionObject(xtype="tth")
+
+    # Attempt to directly modify the property
+    with pytest.raises(
+        AttributeError,
+        match="Direct modification of attribute 'input_xtype' is not allowed. "
+        "Please use 'insert_scattering_quantity' to modify 'input_xtype'.",
+    ):
+        do.input_xtype = "q"
+
+
 def test_copy_object():
     do = DiffractionObject(
         name="test",