Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions news/uuid-rename.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* <news item>

**Changed:**

* DiffractionObject's "id" property renamed to "uuid"

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
14 changes: 7 additions & 7 deletions src/diffpy/utils/diffraction_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class DiffractionObject:
The array containing the quantity of q, tth, d values.
input_xtype : str
The type of the independent variable in `xarray`. Must be one of {*XQUANTITIES}
id : uuid
_uuid : uuid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we are making this attribute public and giving it a getter, I wonder if we should just call it uuid and remove the underscore? What do you think? It may also be less confusing if the attribute name and how it is accessed is the szme?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree. It turns out we shall write a separate docstring for @property methods as shown below (as done in the latest commit):

    @property
    def uuid(self):
        """The unique identifier for the DiffractionObject instance.

        Returns
        -------
        uuid
            The unique identifier of the DiffractionObject instance.
        """
        return self._uuid

This way, User does not have to read private properties starting with _.

API doc is also clearly marked with @property:

Screenshot 2024-12-25 at 11 12 35 PM

The unique identifier for the diffraction object.
Copy link
Contributor Author

@bobleesj bobleesj Dec 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are marked with @property - docstrings are provided separately (please see below)

scat_quantity : str
The type of scattering experiment (e.g., "x-ray", "neutron"). Default is an empty string "".
Expand Down Expand Up @@ -127,7 +127,7 @@ def __init__(
>>> print(do.metadata)
"""

self._id = uuid.uuid4()
self._uuid = uuid.uuid4()
self._input_data(xarray, yarray, xtype, wavelength, scat_quantity, name, metadata)

def _input_data(self, xarray, yarray, xtype, wavelength, scat_quantity, name, metadata):
Expand Down Expand Up @@ -299,12 +299,12 @@ def input_xtype(self, _):
raise AttributeError(_setter_wmsg("input_xtype"))

@property
def id(self):
return self._id
def uuid(self):
return self._uuid

@id.setter
def id(self, _):
raise AttributeError(_setter_wmsg("id"))
@uuid.setter
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while User still has access to uuid, we make it non-settable still.

def uuid(self, _):
raise AttributeError(_setter_wmsg("uuid"))

def get_array_index(self, value, xtype=None):
"""Return the index of the closest value in the array associated with
Expand Down
24 changes: 13 additions & 11 deletions tests/test_diffraction_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ def test_init_valid(do_init_args, expected_do_dict, divide_by_zero_warning_expec
else:
actual_do_dict = DiffractionObject(**do_init_args).__dict__
diff = DeepDiff(
actual_do_dict, expected_do_dict, ignore_order=True, significant_digits=13, exclude_paths="root['_id']"
actual_do_dict, expected_do_dict, ignore_order=True, significant_digits=13, exclude_paths="root['_uuid']"
)
assert diff == {}

Expand Down Expand Up @@ -523,27 +523,29 @@ def test_all_array_setter(do_minimal):
do.all_arrays = np.empty((4, 4))


def test_id_getter(do_minimal):
def test_uuid_getter(do_minimal):
do = do_minimal
assert hasattr(do, "id")
assert isinstance(do.id, UUID)
assert len(str(do.id)) == 36
assert hasattr(do, "uuid")
assert isinstance(do.uuid, UUID)
assert len(str(do.uuid)) == 36


def test_id_getter_with_mock(mocker, do_minimal):
mocker.patch.object(DiffractionObject, "id", new_callable=lambda: UUID("d67b19c6-3016-439f-81f7-cf20a04bee87"))
def test_uuid_getter_with_mock(mocker, do_minimal):
mocker.patch.object(
DiffractionObject, "uuid", new_callable=lambda: UUID("d67b19c6-3016-439f-81f7-cf20a04bee87")
)
do = do_minimal
assert do.id == UUID("d67b19c6-3016-439f-81f7-cf20a04bee87")
assert do.uuid == UUID("d67b19c6-3016-439f-81f7-cf20a04bee87")


def test_id_setter_error(do_minimal):
def test_uuid_setter_error(do_minimal):
do = do_minimal

with pytest.raises(
AttributeError,
match="Direct modification of attribute 'id' is not allowed. Please use 'input_data' to modify 'id'.",
match="Direct modification of attribute 'uuid' is not allowed. Please use 'input_data' to modify 'uuid'.",
):
do.id = uuid.uuid4()
do.uuid = uuid.uuid4()


def test_xarray_yarray_length_mismatch():
Expand Down
Loading