From ee190b9b6c271937f823ed1821a265f6dbcfa4d1 Mon Sep 17 00:00:00 2001 From: Sangjoon Bob Lee Date: Sat, 7 Dec 2024 20:41:19 -0500 Subject: [PATCH 1/9] valid xtype and use getter/setting for xtype --- src/diffpy/utils/diffraction_objects.py | 48 +++++++++++++++++++------ 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/src/diffpy/utils/diffraction_objects.py b/src/diffpy/utils/diffraction_objects.py index 6ce965d0..6316f2a2 100644 --- a/src/diffpy/utils/diffraction_objects.py +++ b/src/diffpy/utils/diffraction_objects.py @@ -26,6 +26,13 @@ def _xtype_wmsg(xtype): ) +def _setter_wmsg(attribute): + return ( + f"Direct modification of attribute '{attribute}' is not allowed." + f"Please use 'insert_scattering_quantity' to modify '{attribute}'.", + ) + + class DiffractionObject: def __init__( self, name=None, wavelength=None, scat_quantity=None, metadata=None, xarray=None, yarray=None, xtype=None @@ -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 xtype(self): + return self._xtype + + @xtype.setter + def xtype(self, _): + raise AttributeError(_setter_wmsg("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._xtype array = self.on_xtype(xtype)[0] if len(array) == 0: raise ValueError(f"The '{xtype}' array is empty. Please ensure it is initialized.") @@ -335,7 +348,7 @@ def insert_scattering_quantity( """ self._set_xarrays(xarray, xtype) self._all_arrays[:, 0] = yarray - self.input_xtype = xtype + self._xtype = xtype # only update these optional values if non-empty quantities are passed to avoid overwriting # valid data inadvertently if metadata: @@ -347,12 +360,25 @@ def insert_scattering_quantity( if wavelength is not None: self.wavelength = wavelength + # 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." + ) + + # 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._xtype in QQUANTITIES: return self.on_q(), "q" - elif self.input_xtype in ANGLEQUANTITIES: + elif self._xtype in ANGLEQUANTITIES: return self.on_tth(), "tth" - elif self.input_xtype in DQUANTITIES: + elif self._xtype in DQUANTITIES: return self.on_d(), "d" def on_q(self): From c119b1f5fc6702ab95fc4fe4b60706e4fb201ab5 Mon Sep 17 00:00:00 2001 From: Sangjoon Bob Lee Date: Sat, 7 Dec 2024 20:41:26 -0500 Subject: [PATCH 2/9] Add tests for getter and setter --- tests/test_diffraction_objects.py | 34 +++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/tests/test_diffraction_objects.py b/tests/test_diffraction_objects.py index aeba1522..141a429a 100644 --- a/tests/test_diffraction_objects.py +++ b/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"I don't know how to handle the xtype, 'invalid_type'. Please rerun specifying an " f"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": "", + "_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": "", + "_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", + "_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", + "_xtype": "d", "name": "", "scat_quantity": "x-ray", "qmin": np.float64(0.0), @@ -407,6 +406,23 @@ def test_all_array_setter(): with pytest.raises( AttributeError, match="Direct modification of attribute 'all_arrays' is not allowed." - "Please use 'insert_scattering_quantity' to modify `all_arrays`.", + "Please use 'insert_scattering_quantity' to modify 'all_arrays'.", ): actual_do.all_arrays = np.empty((4, 4)) + + +def test_xtype_getter(): + do = DiffractionObject(xtype="tth") + assert do.xtype == "tth" + + +def test_xtype_setter(): + do = DiffractionObject(xtype="tth") + + # Attempt to directly modify the property + with pytest.raises( + AttributeError, + match="Direct modification of attribute 'xtype' is not allowed." + "Please use 'insert_scattering_quantity' to modify 'xtype'.", + ): + do.xtype = "q" From 8e993a3db6a9e1555096266ed614ec1fab880598 Mon Sep 17 00:00:00 2001 From: Sangjoon Bob Lee Date: Sat, 7 Dec 2024 20:41:33 -0500 Subject: [PATCH 3/9] Add news --- news/scattering-obj-valid.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 news/scattering-obj-valid.rst diff --git a/news/scattering-obj-valid.rst b/news/scattering-obj-valid.rst new file mode 100644 index 00000000..adf4dfa9 --- /dev/null +++ b/news/scattering-obj-valid.rst @@ -0,0 +1,23 @@ +**Added:** + +* validate xtype belongs to XQUANTITIES during DiffractionObject init + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* From 8431fe262ab8fcd1b75cc4d0ac01c829b79b237f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 9 Dec 2024 16:35:06 +0000 Subject: [PATCH 4/9] [pre-commit.ci] auto fixes from pre-commit hooks --- tests/test_diffraction_objects.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_diffraction_objects.py b/tests/test_diffraction_objects.py index 666f5dbe..82add998 100644 --- a/tests/test_diffraction_objects.py +++ b/tests/test_diffraction_objects.py @@ -427,6 +427,7 @@ def test_xtype_setter(): ): do.xtype = "q" + def test_copy_object(): do = DiffractionObject( name="test", @@ -438,4 +439,3 @@ def test_copy_object(): do_copy = do.copy() assert do == do_copy assert id(do) != id(do_copy) - From b30cc7e1f6905295a7143d12ad04beea6d070bb1 Mon Sep 17 00:00:00 2001 From: Sangjoon Bob Lee Date: Mon, 9 Dec 2024 11:44:09 -0500 Subject: [PATCH 5/9] maintain input_xtype, while make it non-settable --- src/diffpy/utils/diffraction_objects.py | 14 +++++++------- tests/test_diffraction_objects.py | 20 ++++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/diffpy/utils/diffraction_objects.py b/src/diffpy/utils/diffraction_objects.py index 9d4bc323..049bf3ce 100644 --- a/src/diffpy/utils/diffraction_objects.py +++ b/src/diffpy/utils/diffraction_objects.py @@ -198,12 +198,12 @@ def all_arrays(self, _): raise AttributeError(_setter_wmsg("all_arrays")) @property - def xtype(self): - return self._xtype + def input_xtype(self): + return self._input_xtype - @xtype.setter - def xtype(self, _): - raise AttributeError(_setter_wmsg("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 @@ -289,7 +289,7 @@ def get_array_index(self, value, xtype=None): """ if xtype is None: - xtype = self._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.") @@ -348,7 +348,7 @@ def insert_scattering_quantity( """ self._set_xarrays(xarray, xtype) self._all_arrays[:, 0] = yarray - self._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: diff --git a/tests/test_diffraction_objects.py b/tests/test_diffraction_objects.py index 82add998..0384d65d 100644 --- a/tests/test_diffraction_objects.py +++ b/tests/test_diffraction_objects.py @@ -286,7 +286,7 @@ def test_dump(tmp_path, mocker): { "_all_arrays": np.empty(shape=(0, 4)), # instantiate empty "metadata": {}, - "_xtype": "", + "_input_xtype": "", "name": "", "scat_quantity": None, "qmin": np.float64(np.inf), @@ -303,7 +303,7 @@ def test_dump(tmp_path, mocker): { "_all_arrays": np.empty(shape=(0, 4)), "metadata": {"thing": "1", "another": "2"}, - "_xtype": "", + "_input_xtype": "", "name": "test", "scat_quantity": "x-ray", "qmin": np.float64(np.inf), @@ -331,7 +331,7 @@ def test_dump(tmp_path, mocker): ] ), "metadata": {}, - "_xtype": "tth", + "_input_xtype": "tth", "name": "", "scat_quantity": None, "qmin": np.float64(0.0), @@ -360,7 +360,7 @@ def test_dump(tmp_path, mocker): ] ), "metadata": {}, - "_xtype": "d", + "_input_xtype": "d", "name": "", "scat_quantity": "x-ray", "qmin": np.float64(0.0), @@ -411,21 +411,21 @@ def test_all_array_setter(): actual_do.all_arrays = np.empty((4, 4)) -def test_xtype_getter(): +def test_input_xtype_getter(): do = DiffractionObject(xtype="tth") - assert do.xtype == "tth" + assert do.input_xtype == "tth" -def test_xtype_setter(): +def test_input_xtype_setter(): do = DiffractionObject(xtype="tth") # Attempt to directly modify the property with pytest.raises( AttributeError, - match="Direct modification of attribute 'xtype' is not allowed." - "Please use 'insert_scattering_quantity' to modify 'xtype'.", + match="Direct modification of attribute 'input_xtype' is not allowed." + "Please use 'insert_scattering_quantity' to modify 'input_xtype'.", ): - do.xtype = "q" + do.input_xtype = "q" def test_copy_object(): From 2a7428cffa122e95188a9b4ec0a95f952ec81f29 Mon Sep 17 00:00:00 2001 From: Sangjoon Bob Lee Date: Mon, 9 Dec 2024 11:53:29 -0500 Subject: [PATCH 6/9] Use _input_xtype within method --- src/diffpy/utils/diffraction_objects.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/diffpy/utils/diffraction_objects.py b/src/diffpy/utils/diffraction_objects.py index 049bf3ce..2fedf18d 100644 --- a/src/diffpy/utils/diffraction_objects.py +++ b/src/diffpy/utils/diffraction_objects.py @@ -374,11 +374,11 @@ def insert_scattering_quantity( raise ValueError(_xtype_wmsg(xtype)) def _get_original_array(self): - if self._xtype in QQUANTITIES: + if self._input_xtype in QQUANTITIES: return self.on_q(), "q" - elif self._xtype in ANGLEQUANTITIES: + elif self._input_xtype in ANGLEQUANTITIES: return self.on_tth(), "tth" - elif self._xtype in DQUANTITIES: + elif self._input_xtype in DQUANTITIES: return self.on_d(), "d" def on_q(self): From c580d82432f7a86b2ae18a093acec57d225daa03 Mon Sep 17 00:00:00 2001 From: Sangjoon Bob Lee Date: Mon, 9 Dec 2024 11:57:33 -0500 Subject: [PATCH 7/9] Add test for mismatch for y and xarray lenght --- src/diffpy/utils/diffraction_objects.py | 17 +++++++++-------- tests/test_diffraction_objects.py | 10 ++++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/diffpy/utils/diffraction_objects.py b/src/diffpy/utils/diffraction_objects.py index 2fedf18d..3b47dfb2 100644 --- a/src/diffpy/utils/diffraction_objects.py +++ b/src/diffpy/utils/diffraction_objects.py @@ -346,6 +346,15 @@ 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 @@ -360,14 +369,6 @@ def insert_scattering_quantity( if wavelength is not None: self.wavelength = wavelength - # 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." - ) - # Check xtype is valid. An empty string is the default value. if xtype != "": if xtype not in XQUANTITIES: diff --git a/tests/test_diffraction_objects.py b/tests/test_diffraction_objects.py index 0384d65d..88ac38d7 100644 --- a/tests/test_diffraction_objects.py +++ b/tests/test_diffraction_objects.py @@ -411,6 +411,16 @@ def test_all_array_setter(): 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" From 85290a956ad7cc337ab07d18c38dd2e445c0211f Mon Sep 17 00:00:00 2001 From: Sangjoon Bob Lee Date: Mon, 9 Dec 2024 12:01:24 -0500 Subject: [PATCH 8/9] Remove f in the beginning of docstring --- src/diffpy/utils/diffraction_objects.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/diffpy/utils/diffraction_objects.py b/src/diffpy/utils/diffraction_objects.py index 3b47dfb2..5bfb9bd8 100644 --- a/src/diffpy/utils/diffraction_objects.py +++ b/src/diffpy/utils/diffraction_objects.py @@ -392,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 ---------- @@ -428,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 ---------- From 9bf532926f6bd1f2d04a1762f581314a76933921 Mon Sep 17 00:00:00 2001 From: Sangjoon Bob Lee Date: Mon, 9 Dec 2024 12:04:52 -0500 Subject: [PATCH 9/9] Add empty space to _xtype_wmsg --- src/diffpy/utils/diffraction_objects.py | 6 +++--- tests/test_diffraction_objects.py | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/diffpy/utils/diffraction_objects.py b/src/diffpy/utils/diffraction_objects.py index 5bfb9bd8..ab39e53b 100644 --- a/src/diffpy/utils/diffraction_objects.py +++ b/src/diffpy/utils/diffraction_objects.py @@ -21,14 +21,14 @@ 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"Direct modification of attribute '{attribute}' is not allowed. " f"Please use 'insert_scattering_quantity' to modify '{attribute}'.", ) diff --git a/tests/test_diffraction_objects.py b/tests/test_diffraction_objects.py index 88ac38d7..d7b9b32c 100644 --- a/tests/test_diffraction_objects.py +++ b/tests/test_diffraction_objects.py @@ -215,8 +215,8 @@ def test_init_invalid_xtype(): with pytest.raises( ValueError, match=re.escape( - f"I don't know how to handle the xtype, 'invalid_type'. 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, }" ), ): DiffractionObject(xtype="invalid_type") @@ -405,7 +405,7 @@ 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." + 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)) @@ -432,7 +432,7 @@ def test_input_xtype_setter(): # Attempt to directly modify the property with pytest.raises( AttributeError, - match="Direct modification of attribute 'input_xtype' is not allowed." + match="Direct modification of attribute 'input_xtype' is not allowed. " "Please use 'insert_scattering_quantity' to modify 'input_xtype'.", ): do.input_xtype = "q"